Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v1] feat(rtl-arrow): Fix arrows used for CTA's when in RTL #11748

Merged
merged 12 commits into from
May 4, 2024
Merged

[v1] feat(rtl-arrow): Fix arrows used for CTA's when in RTL #11748

merged 12 commits into from
May 4, 2024

Conversation

m4olivei
Copy link
Contributor

@m4olivei m4olivei commented Apr 26, 2024 •
edited
Loading

Related Ticket(s)

Closes #11725
Jira Ticket

Description

Checks document.dir and uses the appropriate arrow icon for the text direction (right in LTR and left in RTL). For many stories involving CTA's, CTA derivatives, or buttons, avoid using the slotted icon when all we're going for is cta-type="local", in other words the arrow icon. In non-CTA cases, there isn't support for a default arrow icon, so we adjust stories to slot the right icon base on document.dir.

Note that in v1, only 'CTA' components have the CTAMixin, whereas it's more broadly used in v2. This means that for non-CTA components its up to the client to slot the right arrow icon (as evidenced by the many *.stories.ts changes). This isn't great, but it felt OK for now 🤷 . Open to ideas however.

Testing instructions

  1. Navigate to any Story making use of a CTA and/or button
  2. Compare LTR and RTL
  3. In LTR, where the arrow icon is used, the arrow should be pointing right
  4. In RTL, where the arrow icon is used, the arrow should be pointing left

Changelog

Changed

  • Change the direction of the CTA arrow when in RTL languages.

@m4olivei m4olivei added bug Something isn't working dev Needs some dev work RTL Assigned to Arabic Right to Left work owner: Innovation Team used when the engineering work will be done by Hybrid Cloud with DDS engineers as consultants test: CDN preview used for generating @carbon/ibmdotcom-web-components CDN for testing v1 labels Apr 26, 2024
@m4olivei m4olivei requested a review from a team as a code owner April 26, 2024 14:15
@m4olivei m4olivei requested review from kennylam and emyarod April 26, 2024 14:15
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: af658f4eeaf2b2f8874601c94b24661252580e86

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Carbon web components (RTL):
https://carbon-web-components-rtl.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Web Components (RTL):
https://ibmdotcom-web-components-rtl.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Apr 26, 2024 •
edited
Loading

Deploy preview created for package Carbon Web Components:
https://carbon-web-components.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

@m4olivei m4olivei requested a review from jkaeser April 26, 2024 17:57
@m4olivei m4olivei assigned andy-blum and unassigned andy-blum Apr 26, 2024
@m4olivei m4olivei requested a review from andy-blum April 26, 2024 17:57
jkaeser
jkaeser approved these changes Apr 30, 2024
View reviewed changes
Copy link
Member

@jkaeser jkaeser left a comment

Choose a reason for hiding this comment

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

LGTM!

packages/web-components/src/components/link-with-icon/link-with-icon.ts Show resolved Hide resolved
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented May 1, 2024 •
edited
Loading

Deploy preview created for package IBM.com Web Components Deploy Preview CDN:
https://ibmdotcom-cdn.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11748/index.html

Built with commit: a9b4f3d314da9fac1bf0d95fdef221fc56121fc1

andy-blum
andy-blum approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@andy-blum andy-blum left a comment

Choose a reason for hiding this comment

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

LGTM

@kennylam kennylam added the Ready to merge Label for the pull requests that are ready to merge label May 3, 2024
@jkaeser jkaeser mentioned this pull request May 3, 2024
Merged
@kennylam kennylam closed this May 3, 2024
@kennylam kennylam reopened this May 3, 2024
@kennylam kennylam closed this May 3, 2024
@kennylam kennylam reopened this May 3, 2024
@m4olivei
Copy link
Contributor Author

m4olivei commented May 3, 2024

@andy-blum I think the jobs that are supposed to be passing are all good now? The ci-check / a11y job is also failing on v1 due to dependency issues looks like.

@kodiakhq kodiakhq bot merged commit be33a5b into carbon-design-system:v1 May 4, 2024
30 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy-blum andy-blum andy-blum approved these changes

@jkaeser jkaeser jkaeser approved these changes

@kennylam kennylam Awaiting requested review from kennylam kennylam is a code owner automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@emyarod emyarod Awaiting requested review from emyarod emyarod is a code owner automatically assigned from carbon-design-system/carbon-for-ibm-com-members

Assignees
No one assigned
Labels
bug Something isn't working dev Needs some dev work owner: Innovation Team used when the engineering work will be done by Hybrid Cloud with DDS engineers as consultants Ready to merge Label for the pull requests that are ready to merge RTL Assigned to Arabic Right to Left work test: CDN preview used for generating @carbon/ibmdotcom-web-components CDN for testing v1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@m4olivei @ibmdotcom-bot @jkaeser @andy-blum @kennylam