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

Nested items in draggable tables are dragged in the opposite left/right direction in RTL #3573

Closed
klonos opened this issue Mar 10, 2019 · 6 comments

Comments

@klonos
Copy link
Member

klonos commented Mar 10, 2019

Description of the bug
It seems that we have somehow missed https://www.drupal.org/project/drupal/issues/197641

Steps To Reproduce

  1. Enable the Language core module
  2. Add an RTL language (Arabic or Hebrew for example), and set that as the site default -> this should switch the site to RTL
  3. Navigate to a page with some nested draggable items (taxonomy terms or menu items).

Actual behavior
Drag an item to the left -> it is dragged to the right 👎
Drag an item to the right -> it is dragged to the left 👎

Expected behavior
Nested draggable items should be dragged to the correct left/right direction.


PR by @klonos: backdrop/backdrop#2529

@herbdool
Copy link

@klonos we should also include this https://www.drupal.org/project/drupal/issues/197641#comment-10396111 as commenter noticed an issue with 2 levels deep.

@herbdool
Copy link

Tested and it looks good. 2+ levels deep.

@quicksketch
Copy link
Member

Padding here is still a little off:

LTR:
image

RTL:
image

Seems like it's the generic padding on [dir="rtl"] ul that may be causing the problem. We may need to add [dir="rtl"] .ckeditor-active-toolbar-configuration { padding: 0; margin: 0; }. Even though it's not different from RTL to LTR, the specificity of [dir="rtl"] ul is winning over the .ckeditor-active-toolbar-configuration selector.

@klonos
Copy link
Member Author

klonos commented Mar 22, 2019

@quicksketch we know, but this is the wrong issue 😛: #3504

Setting this back to RTBC.

@quicksketch
Copy link
Member

Oops! I'm sorry! I'll take another look at this one and make sure I'm looking at the right thing. 😅

@quicksketch
Copy link
Member

Okay, now looking at the right PR, looks great! Thanks @klonos and @herbdool! Merged backdrop/backdrop#2529 into 1.x and 1.12.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants