-
Notifications
You must be signed in to change notification settings - Fork 37
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
STCOM-1150: Remove unchecked calls to Moment.locale #2025
base: b10.3
Are you sure you want to change the base?
Conversation
Reference for those |
Every time I deal with weird time zone/localization bugs, I am reminded of this amazing video |
Ha! Great video, and he sure gets the moral correct (make somebody else do this if possible)! It's no small wonder that date/time pickers seem to always be the missing components in every single OSS component library! I remember learning about unix in college and being totally bowled over by the command |
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.
I believe moment is case-senstive WRT locales so those tests need to check for the exact same locale provided as the prop.
Jira STCOM-1150
This fixes the bug reported in STCOM-1150 and causing UICAL-267 by removing the erroneous call to
moment.locale
.To summarize this Slack thread, the call to
moment.locale
for locales unknown to Moment (such asen-SE
for Swedish English) caused Moment to fallback to default locale settings, however, it would also cache these default settings as the correct locale foren-SE
. WhenTimepicker
later checked to see if Moment knew the locale, Moment then reported that it did, returning incorrectly assumed locale information, resulting in 24-hourTimepicker
s switching to 12-hour at reconstruction.@JohnC-80 / @zburke one of y'all will likely want to clone this branch over to
folio-org
to check CI (I don't have permissions here), however, running tests revealed no new failures (the only date-related failure was due to the change of the space in locale times during a recent Chrome update (replaced with\u202f
)). Fixed that here too, for sanity's sake.