fix lang on relative-time day ordering #175

keithamus · 2022-10-17T09:48:06Z

Tests are failing locally for me, because my locale is en-GB not en-US, which means tests, that depend on method that don't allow for customisation with locale fail.

I've fixed this by making isDayFirst & makeFormatter locale aware (it takes a locale but defaults it to 'default'). Then in relative-time, it calls isDayFirst with the given locale and in local-time it calls makeFormatter with the given locale. Lastly I fixed the locale to en-US in the tests.

Co-authored-by: Kristján Oddsson <koddsson@gmail.com>

primer-css · 2022-10-17T10:09:23Z

👋 Hello and thanks for pinging us! This issue or PR has been added to our inbox and a Design Infrastructure first responder will review it soon.

🎨 If this is a PR that includes a visual change, please make sure to add screenshots in the description or deploy this code to a lab machine with instructions for how to test.
⏩ If this is a PR that includes changes to an interaction, please include a video recording in the description.
⚠️ If this is urgent, please visit us in #primer on Slack and tag the first responders listed in the channel topic.

Co-authored-by: Kristján Oddsson <koddsson@gmail.com>

lukasoppermann · 2022-10-17T10:24:08Z

src/utils.ts

 const dayFirstFormatter = makeFormatter({day: 'numeric', month: 'short'})

 // Private: Determine if the day should be formatted before the month name in
 // the user's current locale. For example, `9 Jun` for en-GB and `Jun 9`
 // for en-US.
 //
 // Returns true if the day appears before the month.
-export function isDayFirst(): boolean {
-  if (dayFirst !== null) {


@keithamus I only medium understand what is happening, but can we safely remove this check? Is this covered by locale in dayFirst because it will just get out of the if if dayFirst === null?

dayFirst cannot be null as it is a const assigned to an object.

lukasoppermann · 2022-10-17T10:26:08Z

Seems legit.

siddharthkp

LGTM!

keithamus requested a review from a team as a code owner October 17, 2022 09:48

fix lang on relative-time day ordering

4985618

Co-authored-by: Kristján Oddsson <koddsson@gmail.com>

keithamus force-pushed the fix-lang-on-relative-time-day-ordering branch from de27411 to 4985618 Compare October 17, 2022 09:48

fix lang on locale-time for some tests

1ecd2cd

Co-authored-by: Kristján Oddsson <koddsson@gmail.com>

keithamus force-pushed the fix-lang-on-relative-time-day-ordering branch from c265f98 to 1ecd2cd Compare October 17, 2022 10:13

lukasoppermann reviewed Oct 17, 2022

View reviewed changes

lukasoppermann approved these changes Oct 17, 2022

View reviewed changes

siddharthkp approved these changes Oct 17, 2022

View reviewed changes

keithamus merged commit c50a703 into main Oct 17, 2022

keithamus deleted the fix-lang-on-relative-time-day-ordering branch October 17, 2022 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix lang on relative-time day ordering #175

fix lang on relative-time day ordering #175

keithamus commented Oct 17, 2022 •

edited

Loading

primer-css commented Oct 17, 2022

lukasoppermann Oct 17, 2022 •

edited

Loading

keithamus Oct 17, 2022

lukasoppermann commented Oct 17, 2022

siddharthkp left a comment

fix lang on relative-time day ordering #175

fix lang on relative-time day ordering #175

Conversation

keithamus commented Oct 17, 2022 • edited Loading

primer-css commented Oct 17, 2022

lukasoppermann Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

keithamus Oct 17, 2022

Choose a reason for hiding this comment

lukasoppermann commented Oct 17, 2022

siddharthkp left a comment

Choose a reason for hiding this comment

keithamus commented Oct 17, 2022 •

edited

Loading

lukasoppermann Oct 17, 2022 •

edited

Loading