Add RTL support #268
Add RTL support #268
Conversation
- Remove old ones that are LTR
Fix some tests to add range assertion
|
|
There was a problem hiding this comment.
There are several minor bugs.
-
On the visual tests there is right indent coming from somewhere on
default-tests-lumo-rtl(could be correlated with the next two things). -
On Edge there is a problem with initial indents for items:
- On IE there is problem with
arrowindents:
It comes from ItemElement already
I guess that’s ok, since the Bottom menu is a bit off comparing to the other items (Plain, Long menu and Items). You can check that the same goes with the current visual test in master:
That seems to be a issue with the current implementation and not introduced on this ticket. Created a ticket for that https://github.com/vaadin/vaadin-context-menu/issues/269 |
Fixed. I splitted the styles so I could remove setting back margins and paddings to initial value. |
| } else if (parent && (parentContentRect.left >= parentContentRect.width)) { | ||
| // Sub-menu is displayed in the left side of root menu If it is nested menu | ||
| style.right = (wdthVport - parentContentRect.right + parentContentRect.width) + 'px'; | ||
| } else if (parent) { // Sub-menu is displayed in the right side of root menu If it is nested menu |
There was a problem hiding this comment.
A bit confused over here: if sub-menu is displayed in the right side of root menu why we are setting right-aligned below. 🤔It means that it's the same as in ltr above displayed in the left side. Should it work this way?
There was a problem hiding this comment.
I think the name right-aligned became a bit misleading now, but I didn't want to touch it, as it could somehow be a breaking change. But basically, from what I could understand, that attribute means that the context-menu will open a sub-menu towards the opposite direction of the "natural flow". So, in the sense of the rtl, right-aligned means left-aligned (😅).
There was a problem hiding this comment.
We can actually consider renaming that one to smth like end-aligned | aligned-end. Not sure if it is a blocker, but we can at least ask from the team :)
- right-aligned is misleading in RTL mode. Change to end-aligned - Still use 'right-aligned' along in LTR - Mark right-aligned as deprecated - Change __isRTL to be a getter instead of method
Fix #267