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

Drawer slide-out direction should be reversed in RTL mode #784

Closed
oliversalzburg opened this issue Jun 9, 2022 · 4 comments
Closed

Drawer slide-out direction should be reversed in RTL mode #784

oliversalzburg opened this issue Jun 9, 2022 · 4 comments
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@oliversalzburg
Copy link
Contributor

Describe the bug

When in RTL mode, a drawer set to placement="start" will slide out from the same direction as when in LTR mode. I believe it should slide out from the opposite side.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://plantdb.net/nursery/
  2. Click on the language selector in the top right and switch to HE.
  3. The drawer slide out icon button is now positioned in the top right.
  4. Click the drawer slide out icon.

Screenshots

image

Browser / OS

  • OS: Windows
  • Browser: Chrome
  • Browser version: 102.0.5005.63
@oliversalzburg oliversalzburg added the bug Things that aren't working right in the library. label Jun 9, 2022
@claviska
Copy link
Member

claviska commented Jun 9, 2022

Good call. It should be an easy fix. I can get to it tomorrow if nobody and it’s a PR in the meantime.

@oliversalzburg
Copy link
Contributor Author

oliversalzburg commented Jun 9, 2022

Maybe .sl-toast-stack needs to have the default position logic reversed in RTL as well.

I'll put that in another ticket. Doesn't seem trivial to me.

@claviska
Copy link
Member

claviska commented Jun 9, 2022

This was a bit more challenging due to the way animations are defined. Since there's no logical property for translateX, the Animation Registry needed to be refactored to support directionality.

Most of the existing animations don't require a RTL alternative, but customizations could easily require it so I've updated drawer and all other components to support this. I also made dir a required option for getAnimation() to ensure we can't forget it in future animations.

In short, ElementAnimation now supports an optional rtlKeyframes property that will be used when necessary. If rtlKeyframes isn't provided, the registry falls back to keyframes. This allows us to support a simple animation definition for most components, while allowing RTL customizations for those that use CSS properties without logical equivalents.

If anyone is interested in how all this works, refer to the Animation Registry utility: https://github.com/shoelace-style/shoelace/blob/next/src/utilities/animation-registry.ts

@oliversalzburg
Copy link
Contributor Author

Wow. That is amazing. Excellent work. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

2 participants