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

fix(datetime): account for 12AM with min times and 12 hour format #25952

Merged
merged 10 commits into from
Sep 19, 2022
4 changes: 2 additions & 2 deletions core/src/components/datetime/test/data.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('generateTime()', () => {
};
const { hours, minutes } = generateTime(today, 'h12', min);

expect(hours.length).toEqual(11);
expect(hours.length).toEqual(10);
expect(minutes.length).toEqual(60);
});
it('should not filter according to min if not on reference day', () => {
Expand Down Expand Up @@ -103,7 +103,7 @@ describe('generateTime()', () => {
};
const { hours, minutes } = generateTime(today, 'h12', undefined, max);

expect(hours.length).toEqual(7);
expect(hours.length).toEqual(8);
expect(minutes.length).toEqual(45);
});
it('should not filter according to min if not on reference day', () => {
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/datetime/test/format.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('getMonthAndDay()', () => {
describe('getFormattedHour()', () => {
it('should only add padding if using 24 hour time', () => {
expect(getFormattedHour(0, true)).toEqual('00');
expect(getFormattedHour(0, false)).toEqual('0');
expect(getFormattedHour(0, false)).toEqual('12');

expect(getFormattedHour(10, true)).toEqual('10');
expect(getFormattedHour(10, false)).toEqual('10');
Expand Down
46 changes: 46 additions & 0 deletions core/src/components/datetime/test/minmax/datetime.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,4 +188,50 @@ test.describe('datetime: minmax', () => {

await expect(datetimeMonthDidChange).toHaveReceivedEventTimes(1);
});

test('should not include 12AM when minimum is greater than 12AM', async ({ page, skip }) => {
skip.rtl();

test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/25183',
});

await page.setContent(`
<ion-datetime
presentation="time"
min="2022-04-25T08:30:00"
max="2022-04-25T21:30:00"
value="2022-04-25T08:30:00"
></ion-datetime>
`);

const hourPickerItems = page.locator(
'ion-datetime ion-picker-column-internal:first-of-type .picker-item:not(.picker-item-empty)'
);
await expect(hourPickerItems).toHaveText(['8', '9', '10', '11']);
});

test('should include 12PM when minimum is greater than 12', async ({ page, skip }) => {
skip.rtl();

test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/25183',
});

await page.setContent(`
<ion-datetime
locale="en-US"
presentation="time"
min="2022-07-29T08:00:00"
value="2022-07-29T12:00:00"
></ion-datetime>
`);

const hourPickerItems = page.locator(
'ion-datetime ion-picker-column-internal:first-of-type .picker-item:not(.picker-item-empty)'
);
await expect(hourPickerItems).toHaveText(['12', '1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11']);
});
});
2 changes: 1 addition & 1 deletion core/src/components/datetime/utils/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const minutes = [
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59,
];
const hour12 = [12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
const hour12 = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
const hour23 = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23];

/**
Expand Down
22 changes: 16 additions & 6 deletions core/src/components/datetime/utils/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,26 @@ export const addTimePadding = (value: number): string => {
};

/**
* Formats the hour value so that it
* is always 2 digits. Only applies
* if using 12 hour format.
* Formats 24 hour times so that
* it always has 2 digits. For
* 12 hour times it ensures that
* hour 0 is formatted as '12'.
*/
export const getFormattedHour = (hour: number, use24Hour: boolean): string => {
if (!use24Hour) {
return hour.toString();
if (use24Hour) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I flipped the order around so that we check positive cases first (IMO I think the function is easier to understand as a result)

return addTimePadding(hour);
}

return addTimePadding(hour);
/**
* If using 12 hour
* format, make sure hour
* 0 is formatted as '12'.
*/
if (hour === 0) {
return '12';
}

return hour.toString();
};

/**
Expand Down