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

Implement icons mirroring with CSS #11462

Closed
wants to merge 3 commits into from

Conversation

thibaudcolas
Copy link
Member

Fixes #8127. This implements icons mirroring with the simplest setup, and does a bit of cleanup so said setup is easily previewable in our styleguide.

Follow-up

I wanted to implement a better approach but there are compatibility issues between our "SVG symbols sprite" approach to icons and how directionality is passed through shadow DOM in HTML. As such, there are two icons left outstanding:

  • tasks, where the "list bullets" and "list text lines" should be reversed.
  • list-ol, also where the "list bullets" and "list text lines" should be reversed.

I was hoping we could create those effects with just CSS, but it seems like we’d need separate alternative icons and a way to switch. I’ve left those icons as-is, so I can create a follow-up issue and hopefully we can find a creative solution.

It’d also be nice if people customizing Wagtail could add their own icons, with RTL support, without having to write CSS. Instead find a way to define the icon needs mirroring directly in the markup. But that’s running into the same issues.

Testing this

To test this, the simplest way is to view the styleguide page in Arabic and make sure all icons are mirrored as appropriate. Here is the v5.2 styleguide in arabic for reference.

I would also recommend to manually edit the "rtl icons" style declaration in DevTools to add an outline as debug styles. Makes it much simpler to check which icons are reversed.

Please check the following:

  • Do the tests still pass?[^1]
  • Does the code comply with the style guide?
  • [ ] For Python changes: Have you added tests to cover the new/fixed behaviour?
  • For front-end changes: Did you test on all of Wagtail’s supported environments?[^2]
    • Please list the exact browser and operating system versions you tested: Safari 17, Chrome 121, Firefox 121 on macOS 14
    • Please list which assistive technologies [^3] you tested: None
  • [ ] For new features: Has the documentation been updated accordingly?

@thibaudcolas thibaudcolas added this to the 6.0 milestone Jan 17, 2024
Copy link

squash-labs bot commented Jan 17, 2024

Manage this branch in Squash

Test this branch here: https://thibaudcolas8127-rtl-mirroring-gdgbh.squash.io

- Set the `viewBox="..."` attribute
- Set the `id="icon-<name>"` attribute, icons are referenced by this name.
- Set the `xmlns="http://www.w3.org/2000/svg"` attribute.
- Set the `viewBox="..."` attribute, and no `width` and `height` attributes.
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed this on the sliders icon. This isn’t that big of a problem, just done for consistency.

Comment on lines +107 to +127
.icon-arrow-right-full,
.icon-arrow-left,
.icon-arrow-right,
.icon-breadcrumb-expand,
.icon-comment,
.icon-comment-add,
.icon-comment-add-reversed,
.icon-cut,
.icon-edit,
.icon-expand-right,
.icon-list-ul,
.icon-lock-open,
.icon-login,
.icon-logout,
.icon-openquote,
.icon-pilcrow,
.icon-redirect,
.icon-resubmit,
.icon-snippet,
.icon-tag,
.icon-thumbtack-crossed {
Copy link
Member

Choose a reason for hiding this comment

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

Wondered whether we can use a generic CSS class and apply it to the files individually instead of explicitly including each directional icon here, turns out we need to apply the styles to the paths instead, and also use transform-origin since the transform will be done relative to the viewBox. Done in #11466

@thibaudcolas thibaudcolas deleted the 8127-rtl-mirroring branch January 18, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement icons mirroring of directional icons for RTL languages support
2 participants