Skip to content

Conversation

@ahayes91
Copy link
Contributor

@ahayes91 ahayes91 commented Jan 20, 2026

Summary

Fixes: #3693

While this issue is fixed in v14, ideally we would have a fix for it in v13 too for teams that are still transitioning!

Release Category

Components


Checklist

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

modules/react/button/lib/ExternalHyperlink.tsx is the entry point!

Areas for Feedback? (optional)

  • Code
  • Documentation
  • Testing
  • Codemods

Testing Manually

I'm having trouble even getting the support branch working locally - when I'm definitely on the node version in the nvmrc and run yarn, I see this error (used npm install -g yarn):

error lerna@9.0.3: The engine "node" is incompatible with this module. Expected version "^20.19.0 || ^22.12.0 || >=24.0.0". Got "18.20.0"
error Found incompatible module.

Do other folks see that?

In any case, I got around it and got storybook running (had to do yarn clean to shake off a few cobwebs) and the icon is flipped now.

Screenshots or GIFs (if applicable)

Screenshot 2026-01-21 at 10 11 20 Screenshot 2026-01-21 at 10 11 37

Thank You Gif (optional)

Elmo poking his head through a hole in a door and saying hi

@ahayes91 ahayes91 changed the title fix: add shouldMirror prop to ExternalHyperlink component for RTL support fix: Add shouldMirror prop to ExternalHyperlink component for RTL support Jan 20, 2026
@cypress
Copy link

cypress bot commented Jan 20, 2026

Workday/canvas-kit    Run #10085

Run Properties:  status check passed Passed #10085  •  git commit 95369dab5c ℹ️: Merge 20738f2af7df6adb5320fa5dee57b7b3acf7c588 into dd23623f3c906804e2783d32c1f3...
Project Workday/canvas-kit
Branch Review aislinn/external-hyperlink-icons
Run status status check passed Passed #10085
Run duration 09m 13s
Commit git commit 95369dab5c ℹ️: Merge 20738f2af7df6adb5320fa5dee57b7b3acf7c588 into dd23623f3c906804e2783d32c1f3...
Committer Aislinn Hayes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 936
View all changes introduced in this branch ↗︎
UI Coverage  20.54%
  Untested elements 1441  
  Tested elements 370  
Accessibility  99.28%
  Failed rules  6 critical   5 serious   0 moderate   2 minor
  Failed elements 98  

export const ExternalLinkRTL = () => (
<CanvasProvider theme={{canvas: {direction: ContentDirection.RTL}}}>
<ExternalHyperlink href="https://workday.com" iconLabel="Opens link in new window" shouldMirror>
השועל החום המהיר קופץ מעל הכלב העצל
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth getting a Hebrew expert to validate this one but according to google it's an almost direct translation of "The quick brown fox jumps over the lazy dog" 😂

@ahayes91 ahayes91 marked this pull request as ready for review January 20, 2026 10:52
@ahayes91 ahayes91 changed the title fix: Add shouldMirror prop to ExternalHyperlink component for RTL support fix: Add shouldMirror prop to ExternalHyperlink component for RTL support to v13 Jan 20, 2026
@mannycarrera4
Copy link
Contributor

Ah i think the nvmrc is lying to you, we recently updated things to 22.x apologies

* If set to `true`, transform the SVG's x-axis to mirror the graphic.
* @default false
*/
shouldMirror?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of adding a new prop, I think we can just update the style object below to add:

':dir(rtl)': {
        transform: 'rotate(270deg)',
      },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d'oh - i swear i did try this originally but turns out I just put it in the wrong place! 😂 thank you, much better!

@ahayes91 ahayes91 changed the title fix: Add shouldMirror prop to ExternalHyperlink component for RTL support to v13 fix: Add RTL icon support to ExternalHyperlink component in v13 Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants