-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
fix: position of side menu controllers in rtl
#922
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@zaaakher is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
Thanks for your PR @zaaakher. I'm curious about the root cause of this. If you set According to the docs: (https://floating-ui.com/docs/usefloating#placement)
Could it be the root cause of this issue is somewhere else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see comment above)
@YousefED you're right, it should ideally adapt to RTL without explicit dir prop or any prop-drilling. After a little digging, it seems that the When I comment out this line
The |
Digging into the source-code of floating-ui, I think this is the root cause of the issue: I think it should be |
Currently `floatingStyles` returned from `useFloating` is overwriting placement styles and preventing `-start` to truly adapt to RTL layouts. See TypeCellOS/BlockNote#922 (comment)
It seems that Based on the response from floating-ui here floating-ui/floating-ui#2982 (comment) it seems the expected automatic direction flip based on dir layout isn't yet supported natively in floating-ui. Other than the minor prop-drilling suggested in this PR, I can't honestly think of an alternative way to do it. 😅 |
This PR simply makes the side menu buttons to be in the correct position when layout is RTL.
Before
After