Skip to content

fix: Normalize literal before dayPeriod in formatToParts #8826

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Normalize literal before dayPeriod in formatToParts #8826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/@internationalized/date/src/DateFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,18 @@ export class DateFormatter implements Intl.DateTimeFormat {

/** Formats a date to an array of parts such as separators, numbers, punctuation, and more. */
formatToParts(value: Date): Intl.DateTimeFormatPart[] {
return this.formatter.formatToParts(value);
const parts = this.formatter.formatToParts(value);

const literalBeforeDayPeriodIndex =
parts.findIndex((p) => p.type === 'dayPeriod') - 1;
if (literalBeforeDayPeriodIndex >= 0) {
const normalizedParts = parts.map((p, i) =>
i === literalBeforeDayPeriodIndex ? {...p, value: ' '} : p
);
return normalizedParts;
}

return parts;
}

/** Formats a date range as a string. */
Expand Down
11 changes: 11 additions & 0 deletions packages/@internationalized/date/tests/DateFormatter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ describe('DateFormatter', function () {
]);
});

it('should format to parts with space as literal before am/pm', function () {
let formatter = new DateFormatter('en-US', {timeStyle: 'short'});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR
I'm a little worried we're going to be playing whack-a-mole with this. I think it'd be better if we could somehow allow suppressHydrationWarning through or hold off rendering for a cycle on server and client when SSR.

Now on to findings:

Many countries prefer the non-breaking space https://www.stylemanual.gov.au/grammar-punctuation-and-conventions/numbers-and-measurements/dates-and-time
Across our supported locales:
In Chrome, it would appear that the non-breaking space char 8239 is the more commonly used.
In Safari, it's overwhelmingly char 32, except for ar-AE which uses char 160.
In Firefox, it's only char 32.

So I think we are ok to default it to 32 for now. But it looks like more than just Node is going to need a fix.

I got this from running the script provided in the issue

['ar-AE', 'bg-BG', 'cs-CZ', 'da-DK',
      'de-DE', 'el-GR', 'en-US', 'es-ES',
      'et-EE', 'fi-FI', 'fr-FR', 'he-IL',
      'hr-HR', 'it-IT', 'ja-JP', 'ko-KR',
      'lt-LT', 'lv-LV', 'nb-NO', 'nl-NL',
      'pl-PL', 'pt-BR', 'pt-PT', 'ro-RO',
      'ru-RU', 'sk-SK', 'sl-SI', 'sr-SP',
      'sv-SE', 'tr-TR', 'uk-UA', 'zh-CN',
      'zh-TW'].map((locale) => {
const formatter = new Intl.DateTimeFormat(locale, { timeStyle: 'long', hour12: true });
const segments = formatter.formatToParts(new Date())
const secondsSegmentIndex = segments.findIndex(segment => segment.type === 'dayPeriod');
          if (secondsSegmentIndex > 0) {
    return segments[secondsSegmentIndex-1].value.charCodeAt(0);
          }
          return 'not used'
})

This should be tested across every locale we support. I also noticed that the test uses timeStyle short, but the example script used long. We would also need to use hour12: true in the options since many locales default to a 24hr representation.
Something along these lines with the right assertions (not just a non-null check).

  it.each(['ar-AE', 'bg-BG', 'cs-CZ', 'da-DK',
    'de-DE', 'el-GR', 'en-US', 'es-ES',
    'et-EE', 'fi-FI', 'fr-FR', 'he-IL',
    'hr-HR', 'it-IT', 'ja-JP', 'ko-KR',
    'lt-LT', 'lv-LV', 'nb-NO', 'nl-NL',
    'pl-PL', 'pt-BR', 'pt-PT', 'ro-RO',
    'ru-RU', 'sk-SK', 'sl-SI', 'sr-SP',
    'sv-SE', 'tr-TR', 'uk-UA', 'zh-CN',
    'zh-TW'])('should format to parts with space as literal before am/pm', function (locale) {
      let formatter = new DateFormatter(locale, {timeStyle: 'short', hour12: true});
      console.log(locale, formatter.formatToParts(new Date(2020, 1, 3, 13, 0)));
      expect(formatter.formatToParts(new Date(2020, 1, 3, 13, 0))).not.toBeNull();

      let formatter2 = new DateFormatter(locale, {timeStyle: 'long', hour12: true});
      console.log(locale, formatter2.formatToParts(new Date(2020, 1, 3, 13, 0)));
      expect(formatter2.formatToParts(new Date(2020, 1, 3, 13, 0))).not.toBeNull();

      let formatter3 = new DateFormatter(locale, {dateStyle: 'full', timeStyle: 'long', hour12: true});
      console.log(locale, formatter3.formatToParts(new Date(2020, 1, 3, 13, 0)));
      expect(formatter3.formatToParts(new Date(2020, 1, 3, 13, 0))).not.toBeNull();
    });

Good news, it doesn't crash in locales such as zh-TW which actually places the dayPeriod first

    zh-TW [
      { type: 'dayPeriod', value: '下午' },
      { type: 'hour', value: '1' },
      { type: 'literal', value: ':' },
      { type: 'minute', value: '00' },
      { type: 'literal', value: ':' },
      { type: 'second', value: '00' },
      { type: 'literal', value: ' [' },
      { type: 'timeZoneName', value: 'GMT+11' },
      { type: 'literal', value: ']' }
    ]

However, while testing, I ran into an issue ines-ES where the space is inside the dayPeriod itself in addition to outside. In Node, Chrome, and Firefox it's 160, and in Safari it's 32

const formatter = new Intl.DateTimeFormat('es-ES', { timeStyle: 'long', hour12: true });
const segments = formatter.formatToParts(new Date())
console.log(segments[6].value.charCodeAt(2));

expect(formatter.formatToParts(new Date(2020, 1, 3, 13, 0))).toEqual([
{type: 'hour', value: '1'},
{type: 'literal', value: ':'},
{type: 'minute', value: '00'},
{type: 'literal', value: ' '},
{type: 'dayPeriod', value: 'PM'}
]);
});

it('should format a range', function () {
let formatter = new DateFormatter('en-US');
// Test fallback
Expand Down
16 changes: 8 additions & 8 deletions packages/@react-spectrum/datepicker/test/DatePicker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ describe('DatePicker', function () {
);

let combobox = getAllByRole('group')[0];
expect(getTextValue(combobox)).toBe('2/3/2019, 8:45 AM');
expect(getTextValue(combobox)).toBe('2/3/2019, 8:45 AM');

let button = getByRole('button');
await user.click(button);
Expand All @@ -450,15 +450,15 @@ describe('DatePicker', function () {
expect(selected.children[0]).toHaveAttribute('aria-label', 'Sunday, February 3, 2019 selected');

let timeField = getAllByLabelText('Time')[0];
expect(getTextValue(timeField)).toBe('8:45 AM');
expect(getTextValue(timeField)).toBe('8:45 AM');

// selecting a date should not close the popover
await user.click(selected.nextSibling.children[0]);

expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(new CalendarDateTime(2019, 2, 4, 8, 45));
expect(getTextValue(combobox)).toBe('2/4/2019, 8:45 AM');
expect(getTextValue(combobox)).toBe('2/4/2019, 8:45 AM');

let hour = within(timeField).getByLabelText('hour,');
expect(hour).toHaveAttribute('role', 'spinbutton');
Expand All @@ -472,7 +472,7 @@ describe('DatePicker', function () {
expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveBeenCalledWith(new CalendarDateTime(2019, 2, 4, 9, 45));
expect(getTextValue(combobox)).toBe('2/4/2019, 9:45 AM');
expect(getTextValue(combobox)).toBe('2/4/2019, 9:45 AM');
});

it('should not throw error when deleting values from time field when CalendarDateTime value is used', async function () {
Expand All @@ -484,7 +484,7 @@ describe('DatePicker', function () {
);

let combobox = getAllByRole('group')[0];
expect(getTextValue(combobox)).toBe('2/3/2019, 10:45 AM');
expect(getTextValue(combobox)).toBe('2/3/2019, 10:45 AM');

let button = getByRole('button');
await user.click(button);
Expand All @@ -497,15 +497,15 @@ describe('DatePicker', function () {
expect(selected.children[0]).toHaveAttribute('aria-label', 'Sunday, February 3, 2019 selected');

let timeField = getAllByLabelText('Time')[0];
expect(getTextValue(timeField)).toBe('10:45 AM');
expect(getTextValue(timeField)).toBe('10:45 AM');

// selecting a date should not close the popover
await user.click(selected.nextSibling.children[0]);

expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(new CalendarDateTime(2019, 2, 4, 10, 45));
expect(getTextValue(combobox)).toBe('2/4/2019, 10:45 AM');
expect(getTextValue(combobox)).toBe('2/4/2019, 10:45 AM');

let hour = within(timeField).getByLabelText('hour,');
expect(hour).toHaveAttribute('role', 'spinbutton');
Expand All @@ -521,7 +521,7 @@ describe('DatePicker', function () {
expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveBeenCalledWith(new CalendarDateTime(2019, 2, 4, 1, 45));
expect(getTextValue(combobox)).toBe('2/4/2019, 1:45 AM');
expect(getTextValue(combobox)).toBe('2/4/2019, 1:45 AM');
});

it('should fire onChange until both date and time are selected', async function () {
Expand Down
20 changes: 10 additions & 10 deletions packages/@react-spectrum/datepicker/test/DateRangePicker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ describe('DateRangePicker', function () {

let startDate = getByTestId('start-date');
let endDate = getByTestId('end-date');
expect(getTextValue(startDate)).toBe('2/3/2019, 8:45 AM');
expect(getTextValue(endDate)).toBe('5/6/2019, 10:45 AM');
expect(getTextValue(startDate)).toBe('2/3/2019, 8:45 AM');
expect(getTextValue(endDate)).toBe('5/6/2019, 10:45 AM');

let button = getByRole('button');
await user.click(button);
Expand All @@ -506,10 +506,10 @@ describe('DateRangePicker', function () {
expect(selected.children[0]).toHaveAttribute('aria-label', 'Selected Range: Sunday, February 3 to Monday, May 6, 2019, Sunday, February 3, 2019 selected');

let startTimeField = getAllByLabelText('Start time')[0];
expect(getTextValue(startTimeField)).toBe('8:45 AM');
expect(getTextValue(startTimeField)).toBe('8:45 AM');

let endTimeField = getAllByLabelText('End time')[0];
expect(getTextValue(endTimeField)).toBe('10:45 AM');
expect(getTextValue(endTimeField)).toBe('10:45 AM');

// selecting a date should not close the popover
await user.click(getByLabelText('Sunday, February 10, 2019 selected'));
Expand All @@ -518,8 +518,8 @@ describe('DateRangePicker', function () {
expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith({start: new CalendarDateTime(2019, 2, 10, 8, 45), end: new CalendarDateTime(2019, 2, 17, 10, 45)});
expect(getTextValue(startDate)).toBe('2/10/2019, 8:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 10:45 AM');
expect(getTextValue(startDate)).toBe('2/10/2019, 8:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 10:45 AM');

let hour = within(startTimeField).getByLabelText('hour,');
expect(hour).toHaveAttribute('role', 'spinbutton');
Expand All @@ -534,8 +534,8 @@ describe('DateRangePicker', function () {
expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveBeenCalledWith({start: new CalendarDateTime(2019, 2, 10, 9, 45), end: new CalendarDateTime(2019, 2, 17, 10, 45)});
expect(getTextValue(startDate)).toBe('2/10/2019, 9:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 10:45 AM');
expect(getTextValue(startDate)).toBe('2/10/2019, 9:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 10:45 AM');

hour = within(endTimeField).getByLabelText('hour,');
expect(hour).toHaveAttribute('role', 'spinbutton');
Expand All @@ -550,8 +550,8 @@ describe('DateRangePicker', function () {
expect(dialog).toBeVisible();
expect(onChange).toHaveBeenCalledTimes(3);
expect(onChange).toHaveBeenCalledWith({start: new CalendarDateTime(2019, 2, 10, 9, 45), end: new CalendarDateTime(2019, 2, 17, 11, 45)});
expect(getTextValue(startDate)).toBe('2/10/2019, 9:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 11:45 AM');
expect(getTextValue(startDate)).toBe('2/10/2019, 9:45 AM');
expect(getTextValue(endDate)).toBe('2/17/2019, 11:45 AM');
});

it('should not fire onChange until both date range and time range are selected', async function () {
Expand Down