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.

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

refactor(@carbon/styles): use logical properties for better RTL support #14531

Conversation

tw15egan
Copy link
Collaborator

@tw15egan tw15egan commented Aug 29, 2023

Closes #13619

Updates the repo to use CSS Logical Properties. This will help users render the components in RTL mode without the need for post-processors like RTLCSS

Changelog

New

  • stylelint-use-logical added to stylelint-config-carbon
  • Enabled linting to ensure logical properties are used going forward

Changed

  • Updated all layout styles to use logical properties rather than left, right, top, etc...

Testing / Reviewing

Ensure there are no visual regressions. Select rtl via the Text direction dropdown via storybook and ensure components render as expected.

I have encountered one issue with Popover, which we can address in a separate PR. Since we use bottom-right, top-left etc, these styles are broken when RTL mode is enabled

@tw15egan tw15egan force-pushed the use-logical-properties branch from 79c3b16 to a24b733 Compare August 29, 2023 13:57
@netlify
Copy link

netlify bot commented Aug 29, 2023

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 79c3b16
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/64edf8d2cceb2b0008b6c970
😎 Deploy Preview https://deploy-preview-14531--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 29, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 79c3b16
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/64edf8d37995810008a822d6
😎 Deploy Preview https://deploy-preview-14531--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 29, 2023

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 6d573cd
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/64ee3dc5c8f7c90008beb707
😎 Deploy Preview https://deploy-preview-14531--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 29, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 6d573cd
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/64ee3dc515931300090661fd
😎 Deploy Preview https://deploy-preview-14531--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tw15egan tw15egan marked this pull request as ready for review August 29, 2023 17:08
@tw15egan tw15egan requested a review from a team as a code owner August 29, 2023 17:08
@alisonjoseph
Copy link
Member

This looks great! Had a couple questions around what components should look like in rtl, these may be unrelated to this PR

Should accordion title be right aligned?
Screenshot 2023-08-29 at 2 09 23 PM

  • Breadcrumb with overflow
Screenshot 2023-08-29 at 2 11 06 PM
  • Button
Screenshot 2023-08-29 at 2 11 38 PM
  • Button set is missing the border
Screenshot 2023-08-29 at 2 12 06 PM
  • Checkbox missing spacing
Screenshot 2023-08-29 at 2 17 15 PM
  • Codesnippet
Screenshot 2023-08-29 at 2 17 41 PM Screenshot 2023-08-29 at 2 18 09 PM
  • Combobox
Screenshot 2023-08-29 at 2 18 56 PM
  • Datatable missing spacing to the right of text
Screenshot 2023-08-29 at 2 21 34 PM same issue with skeleton state Screenshot 2023-08-29 at 2 23 13 PM
  • Spacing seems off on Dropdown
Screenshot 2023-08-29 at 2 24 15 PM
  • Inline dropdown
Screenshot 2023-08-29 at 2 24 40 PM
  • Icon button
Screenshot 2023-08-29 at 2 25 30 PM
  • should the ... be after for inline loading?
Screenshot 2023-08-29 at 2 25 54 PM
  • Is menu correct?
Screenshot 2023-08-29 at 2 26 41 PM
  • MenuButton
Screenshot 2023-08-29 at 2 27 26 PM

... will keep reviewing starting with Modal later

@tw15egan
Copy link
Collaborator Author

tw15egan commented Aug 29, 2023

@alisonjoseph anything that uses Tooltip or Popover will be broken in RTL 😬 . I'm more concerned with making sure the components aren't broken in LTR. I think it might be best to fix the RTL-specific issues in a separate PR, what do you think? I just don't want to make this PR any harder to review`

@alisonjoseph
Copy link
Member

@tw15egan Agree this is already a huge PR. My only worry is that if teams are using RTL, and then we release this they will have broken components. I wonder if we could just keep this branch and open PR's to fix things into this branch, and then release all at once?

@tw15egan
Copy link
Collaborator Author

Good point, I guess we should create a new branch and we can work off that until we can fix these issues

@tw15egan tw15egan changed the base branch from main to logical-properties August 30, 2023 13:05
@tw15egan tw15egan force-pushed the use-logical-properties branch from 6d573cd to 7964324 Compare August 30, 2023 13:11
@tw15egan
Copy link
Collaborator Author

@alisonjoseph this is now pointing to a new logical-properties branch, should we merge this and push new fixes for the RTL issues to that branch? We can create an issue for outstanding issues we come across

@alisonjoseph
Copy link
Member

@tw15egan sounds good, lets merge it 👍

@tw15egan tw15egan merged commit b6fedd7 into carbon-design-system:logical-properties Aug 30, 2023
tw15egan added a commit that referenced this pull request Sep 5, 2023
…rt (#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same
tw15egan added a commit that referenced this pull request Sep 11, 2023
…rt (#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same
github-merge-queue bot pushed a commit that referenced this pull request Sep 11, 2023
…upport (#14580)

* refactor(@carbon/styles): use logical properties for better RTL support (#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same

* [WIP] fix(components): fix RTL issues with components (#14546)

* fix(components): fix RTL issues with components

* style(components): more fixes for RTL styles

* style(components): more fixes for RTL styles

* fix(Popover): fix popover styles in RTL mode

* fix(TreeView): fix RTL issues with TreeView

* fix(Breadcrumb): fix small issue with overflow menu variant

* fix(Popover): fix autoalign story issues

* fix(menuButton): fix RTL issues with MenuButton (#14565)

* fix(menuButton): fix RTL issues with MenuButton

* fix(Menu): adjust caret rotation

* refactor(menu): remove rtl override

* fix(Menu): adjust story positioning, add CaretLeft for RTL mode

* refactor(Menu): useLayoutContext to check if parent has direction set

* style(React): fix linting issues with storybook styles

* test(Accordion): update snapshots

* fix(Tooltip): move border
github-merge-queue bot pushed a commit that referenced this pull request Sep 12, 2023
* refactor(@carbon/styles): use logical properties for better RTL support (#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same

* [WIP] fix(components): fix RTL issues with components (#14546)

* fix(components): fix RTL issues with components

* style(components): more fixes for RTL styles

* style(components): more fixes for RTL styles

* fix(Popover): fix popover styles in RTL mode

* fix(TreeView): fix RTL issues with TreeView

* fix(Breadcrumb): fix small issue with overflow menu variant

* fix(Popover): fix autoalign story issues

* fix(menuButton): fix RTL issues with MenuButton (#14565)

* fix(menuButton): fix RTL issues with MenuButton

* fix(Menu): adjust caret rotation

* refactor(menu): remove rtl override

* fix(Menu): adjust story positioning, add CaretLeft for RTL mode

* refactor(Menu): useLayoutContext to check if parent has direction set

* style(React): fix linting issues with storybook styles

* test(Accordion): update snapshots

* fix(Tooltip): move border

* fix(components): fix a few more RTL issues with components

* fix(Slider): adjust positioning logic when rtl mode is enabled

* chore(test): remove console.log

* test(Slider): update tests

* fix(ProgressBar): reverse indeterminate animation in RTL mode
misiekhardcore pushed a commit to misiekhardcore/carbon that referenced this pull request Sep 18, 2023
…upport (carbon-design-system#14580)

* refactor(@carbon/styles): use logical properties for better RTL support (carbon-design-system#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same

* [WIP] fix(components): fix RTL issues with components (carbon-design-system#14546)

* fix(components): fix RTL issues with components

* style(components): more fixes for RTL styles

* style(components): more fixes for RTL styles

* fix(Popover): fix popover styles in RTL mode

* fix(TreeView): fix RTL issues with TreeView

* fix(Breadcrumb): fix small issue with overflow menu variant

* fix(Popover): fix autoalign story issues

* fix(menuButton): fix RTL issues with MenuButton (carbon-design-system#14565)

* fix(menuButton): fix RTL issues with MenuButton

* fix(Menu): adjust caret rotation

* refactor(menu): remove rtl override

* fix(Menu): adjust story positioning, add CaretLeft for RTL mode

* refactor(Menu): useLayoutContext to check if parent has direction set

* style(React): fix linting issues with storybook styles

* test(Accordion): update snapshots

* fix(Tooltip): move border
misiekhardcore pushed a commit to misiekhardcore/carbon that referenced this pull request Sep 18, 2023
…em#14635)

* refactor(@carbon/styles): use logical properties for better RTL support (carbon-design-system#14531)

* refactor(styles): update styles to use CSS logical properties

* fix(styles): change padding-inline-end to padding-inline

* fix(styles): run stylelint --fix on components

* fix(styles): run stylelint --fix on utilities

* fix(Select): test padding fix

* chore(stylelint): run styelint on files outside @carbon/styles

* fix(AspectRatio): remove unsupported float: inline-start

* fix(ContainedList): redefine tag tokens in contained list

* fix(ContainedList): keep story the same

* [WIP] fix(components): fix RTL issues with components (carbon-design-system#14546)

* fix(components): fix RTL issues with components

* style(components): more fixes for RTL styles

* style(components): more fixes for RTL styles

* fix(Popover): fix popover styles in RTL mode

* fix(TreeView): fix RTL issues with TreeView

* fix(Breadcrumb): fix small issue with overflow menu variant

* fix(Popover): fix autoalign story issues

* fix(menuButton): fix RTL issues with MenuButton (carbon-design-system#14565)

* fix(menuButton): fix RTL issues with MenuButton

* fix(Menu): adjust caret rotation

* refactor(menu): remove rtl override

* fix(Menu): adjust story positioning, add CaretLeft for RTL mode

* refactor(Menu): useLayoutContext to check if parent has direction set

* style(React): fix linting issues with storybook styles

* test(Accordion): update snapshots

* fix(Tooltip): move border

* fix(components): fix a few more RTL issues with components

* fix(Slider): adjust positioning logic when rtl mode is enabled

* chore(test): remove console.log

* test(Slider): update tests

* fix(ProgressBar): reverse indeterminate animation in RTL mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use CSS Logical Properties and Values for Styling
3 participants