From d0b624454b8c597d5949c8b14d6e944d16b23697 Mon Sep 17 00:00:00 2001 From: Victoria Date: Wed, 22 Jan 2025 15:02:37 +0200 Subject: [PATCH 1/2] fix(datetime): allow accurate typing of time values in 24-hour format - Adjusted the `selectMultiColumn` logic to handle keyboard values like 20 and 22 dynamically. - Introduced checks for the maximum column value to enable flexible input behavior. - Added e2e tests to verify correct value selection for both 12-hour and 24-hour formats. Closes #28877 --- core/src/components/picker/picker.tsx | 32 ++++--- .../picker/test/keyboard-entry/picker.e2e.ts | 94 +++++++++++++++++++ 2 files changed, 115 insertions(+), 11 deletions(-) diff --git a/core/src/components/picker/picker.tsx b/core/src/components/picker/picker.tsx index 1d98e049d4b..d28d04be3c3 100644 --- a/core/src/components/picker/picker.tsx +++ b/core/src/components/picker/picker.tsx @@ -432,6 +432,17 @@ export class Picker implements ComponentInterface { const firstColumn = numericPickers[0]; const lastColumn = numericPickers[1]; + // Get the maximum value from the first column's options + const columnOptions = Array.from(firstColumn.querySelectorAll('ion-picker-column-option')); + const maxFirstColumnValue = Math.max( + ...columnOptions + .map((option) => parseInt(option.textContent?.trim() || '0', 10)) // Extract and parse text content + .filter((num) => !isNaN(num)) // Ensure valid numbers only + ); + + // check for 24-hour format + const allowTwo = maxFirstColumnValue >= 20; + let value = inputEl.value; let minuteValue; switch (value.length) { @@ -439,14 +450,11 @@ export class Picker implements ComponentInterface { this.searchColumn(firstColumn, value); break; case 2: - /** - * If the first character is `0` or `1` it is - * possible that users are trying to type `09` - * or `11` into the hour field, so we should look - * at that first. - */ const firstCharacter = inputEl.value.substring(0, 1); - value = firstCharacter === '0' || firstCharacter === '1' ? inputEl.value : firstCharacter; + value = + firstCharacter === '0' || firstCharacter === '1' || (allowTwo && firstCharacter === '2') + ? inputEl.value + : firstCharacter; this.searchColumn(firstColumn, value); @@ -462,14 +470,14 @@ export class Picker implements ComponentInterface { break; case 3: /** - * If the first character is `0` or `1` it is + * If the first character is `0` or `1` or allowed '2' it is * possible that users are trying to type `09` * or `11` into the hour field, so we should look * at that first. */ const firstCharacterAgain = inputEl.value.substring(0, 1); value = - firstCharacterAgain === '0' || firstCharacterAgain === '1' + firstCharacterAgain === '0' || firstCharacterAgain === '1' || (allowTwo && firstCharacterAgain === '2') ? inputEl.value.substring(0, 2) : firstCharacterAgain; @@ -486,14 +494,16 @@ export class Picker implements ComponentInterface { break; case 4: /** - * If the first character is `0` or `1` it is + * If the first character is `0` or `1` or allowed '2' it is * possible that users are trying to type `09` * or `11` into the hour field, so we should look * at that first. */ const firstCharacterAgainAgain = inputEl.value.substring(0, 1); value = - firstCharacterAgainAgain === '0' || firstCharacterAgainAgain === '1' + firstCharacterAgainAgain === '0' || + firstCharacterAgainAgain === '1' || + (allowTwo && firstCharacterAgainAgain === '2') ? inputEl.value.substring(0, 2) : firstCharacterAgainAgain; this.searchColumn(firstColumn, value); diff --git a/core/src/components/picker/test/keyboard-entry/picker.e2e.ts b/core/src/components/picker/test/keyboard-entry/picker.e2e.ts index 64d4a1cce13..036d80bf54f 100644 --- a/core/src/components/picker/test/keyboard-entry/picker.e2e.ts +++ b/core/src/components/picker/test/keyboard-entry/picker.e2e.ts @@ -163,6 +163,100 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => await expect(ionChange).toHaveReceivedEventDetail({ value: 12 }); await expect(column).toHaveJSProperty('value', 12); }); + + test('should allow typing 22 in a column where the max value is 23 and not just set it to 2', async ({ page }) => { + await page.setContent( + ` + + + + + + `, + config + ); + + const column = page.locator('ion-picker-column'); + const ionChange = await page.spyOnEvent('ionChange'); + await column.evaluate((el: HTMLIonPickerColumnElement) => el.setFocus()); + + // Simulate typing '22' + await page.keyboard.press('Digit2'); + await page.keyboard.press('Digit2'); + + // Ensure the column value is updated to 22 + await expect(ionChange).toHaveReceivedEventDetail({ value: 22 }); + await expect(column).toHaveJSProperty('value', 22); + }); + + test('should set value to 2 and not wait for another digit when max value is 12', async ({ page }) => { + await page.setContent( + ` + + + + + + `, + config + ); + + const column = page.locator('ion-picker-column'); + const ionChange = await page.spyOnEvent('ionChange'); + await column.evaluate((el: HTMLIonPickerColumnElement) => el.setFocus()); + + // Simulate typing '2' + await page.keyboard.press('Digit2'); + + // Ensure the value is immediately set to 2 + await expect(ionChange).toHaveReceivedEventDetail({ value: 2 }); + await expect(column).toHaveJSProperty('value', 2); + }); + test('pressing Enter should dismiss the keyboard', async ({ page }) => { test.info().annotations.push({ type: 'issue', From 6ba39b471fb6fe0a59fda119dae5f48fccf01cde Mon Sep 17 00:00:00 2001 From: Victoria Date: Wed, 22 Jan 2025 15:07:52 +0200 Subject: [PATCH 2/2] add missing comment --- core/src/components/picker/picker.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/src/components/picker/picker.tsx b/core/src/components/picker/picker.tsx index d28d04be3c3..683fb4f8c1e 100644 --- a/core/src/components/picker/picker.tsx +++ b/core/src/components/picker/picker.tsx @@ -449,6 +449,12 @@ export class Picker implements ComponentInterface { case 1: this.searchColumn(firstColumn, value); break; + /** + * If the first character is `0` or `1` or allowed '2' it is + * possible that users are trying to type `09` + * or `11` into the hour field, so we should look + * at that first. + */ case 2: const firstCharacter = inputEl.value.substring(0, 1); value =