fix(datetime): RTL will no longer infinitely scroll #24475
Conversation
| @@ -995,9 +997,12 @@ export class Datetime implements ComponentInterface { | |||
| const nextMonth = calendarBodyRef.querySelector('.calendar-month:last-of-type'); | |||
| if (!nextMonth) { return; } | |||
|
|
|||
| const isRTL = document.dir === 'rtl'; | |||
There was a problem hiding this comment.
We need to check if dir="rtl" was set on the host element as well. Might be good to make this a utility that other components can use as well. Doing a quick search reveals that other components do not check the host when they probably should: https://github.com/ionic-team/ionic-framework/search?l=TSX&p=2&q=rtl
There was a problem hiding this comment.
Added a new util for isRTL.
How do you feel about splitting out a separate issue for each component that has rtl checks only for document? That way the change log will better reflect those changes to the util change and won't be tied to the datetime changes. If you feel good about that, I'm happy to create an issue for each.
There was a problem hiding this comment.
Yep let's log some tech debt for that 👍
averyjohnston
changed the title
|
I'm still seeing the issue on Windows Chrome 🤔 Happens both when setting |
Ahh.. yeah I know the exact reason. The change to fix the lint error. One sec. |
|
Fixed that issue & added a test to cover that condition. |
| export const isRTL = (hostEl?: Pick<HTMLElement, 'dir'>) => { | ||
| if (hostEl) { | ||
| if (hostEl.dir !== '') { | ||
| return hostEl.dir.toLowerCase() === 'rtl'; |
There was a problem hiding this comment.
Should we be converting to lowercase? It looks "Ltr" should be treated as invalid looking at the accepted values: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/dir
There was a problem hiding this comment.
I'm happy to swap that to explicit checks. Was siding on Dx, but if the browser will ignore any values that aren't rtl | ltr | auto, that isn't very helpful to be different.
There was a problem hiding this comment.
Oops I take it back. The browser dir is not case sensitive, so what you have currently aligns with how the browser behaves: https://codepen.io/liamdebeasi/pen/Jjrvgrp
There was a problem hiding this comment.
Haha, that change is reverted 👍
This reverts commit 31fe01f.
|
this isn't fix the problem for all cases. |
|
@sean-perkins This problem still exists in Android 9. Android 12 is ok. |
"@ionic/angular": "^6.0.12" |
Image.from.iOS.movStill facing same issue in IOS. Is anyone know how to fix it? |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm run lint) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
The date picker will infinitely scroll on initialization when using RTL.
Issue Number: Resolves #24472
What is the new behavior?
The calculations for scroll positioning will account for RTL.
Does this introduce a breaking change?
Other information
Kapture.2021-12-22.at.14.08.50.mp4
Kapture.2021-12-22.at.14.07.43.mp4