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

RTL bugs #32327

Closed
3 tasks done
FaridAghili opened this issue Dec 4, 2020 · 3 comments · Fixed by #32436
Closed
3 tasks done

RTL bugs #32327

FaridAghili opened this issue Dec 4, 2020 · 3 comments · Fixed by #32436

Comments

@FaridAghili
Copy link

FaridAghili commented Dec 4, 2020

  • Windows 10

  • Google Chrome 87

  • a weird bug in form-switch:
    image

    Notice the switch circle which isn't vertically centered.
    It only happens in unchecked unfocused state, not when checked or when it has focus.

  • In this url, please take a look at this:
    image

    I guess we should flip this chevrons like this:

    image

    Which can be done with transform: rotate(180deg). Also the transform-origin can be increased to .6em to have more space between chevron and text.

  • Carousel direction is wrong in RTL when changed by keyboard or touch, however left/right icons works correct on click See fix

@FaridAghili FaridAghili changed the title RTL: toggle switch visual bug RTL bugs Dec 4, 2020
@twbs twbs deleted a comment from FaridAghili Dec 4, 2020
@twbs twbs deleted a comment from FaridAghili Dec 4, 2020
@FaridAghili
Copy link
Author

FaridAghili commented Dec 7, 2020

I can confirm first bug is fixed (switch circle)

@FaridAghili
Copy link
Author

About the carousel keyboard and touch which currently changes slide in wrong direction in RTL layout, It can be fixed to behave as expected in both RTL and LTR with very little change:

const ARROW_LEFT_KEY = 'ArrowLeft'

Change to:

const ARROW_LEFT_KEY = isRTL ? 'ArrowRight' : 'ArrowLeft'

const ARROW_RIGHT_KEY = 'ArrowRight'

Change to:

const ARROW_RIGHT_KEY = isRTL ? 'ArrowLeft' : 'ArrowRight'

These 2 changes fix keyboard direction.

Now for fixing touch direction:
1.

this.prev()

Change to:

isRTL ? this.next() : this.prev()

this.next()

Change to

isRTL ? this.prev() : this.next()

Also make sure to import isRTL from util:

// /js/src/carousel.js

import {
  getjQuery,
  onDOMContentLoaded,
  TRANSITION_END,
  emulateTransitionEnd,
  getElementFromSelector,
  getTransitionDurationFromElement,
  isVisible,
+  isRTL,
  reflow,
  triggerTransitionEnd,
  typeCheckConfig
} from './util/index'

@ffoodd @XhmikosR

@XhmikosR
Copy link
Member

XhmikosR commented Dec 7, 2020

You can always make PRs @FaridAghili. But most importantly, we need to add JS tests for the changes in the RTL PR as mentioned in #32330.

@ffoodd ffoodd mentioned this issue Dec 11, 2020
3 tasks
@XhmikosR XhmikosR added this to Inbox in v5.0.0-beta2 via automation Dec 11, 2020
v5.0.0-beta2 automation moved this from Inbox to Done Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.0.0-beta2
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants