Skip to content

Commit

Permalink
fix(datetime): allow accurate typing of time values in 24-hour format
Browse files Browse the repository at this point in the history
- 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 ionic-team#28877
  • Loading branch information
vunizhona committed Jan 22, 2025
1 parent b71f2e9 commit d0b6244
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 11 deletions.
32 changes: 21 additions & 11 deletions core/src/components/picker/picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -432,21 +432,29 @@ 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) {
case 1:
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);

Expand All @@ -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;

Expand All @@ -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);
Expand Down
94 changes: 94 additions & 0 deletions core/src/components/picker/test/keyboard-entry/picker.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
<ion-picker>
<ion-picker-column></ion-picker-column>
</ion-picker>
<script>
const column = document.querySelector('ion-picker-column');
column.numericInput = true;
const items = [
{ text: '01', value: 1 },
{ text: '02', value: 2},
{ text: '20', value: 20 },
{ text: '21', value: 21 },
{ text: '22', value: 22 },
{ text: '23', value: 23 }
];
items.forEach((item) => {
const option = document.createElement('ion-picker-column-option');
option.value = item.value;
option.textContent = item.text;
column.appendChild(option);
});
</script>
`,
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(
`
<ion-picker>
<ion-picker-column></ion-picker-column>
</ion-picker>
<script>
const column = document.querySelector('ion-picker-column');
column.numericInput = true;
const items = [
{ text: '01', value: 1 },
{ text: '02', value: 2 },
{ text: '03', value: 3 },
{ text: '04', value: 4 },
{ text: '05', value: 5 },
{ text: '06', value: 6 },
{ text: '07', value: 7 },
{ text: '08', value: 8 },
{ text: '09', value: 9 },
{ text: '10', value: 10 },
{ text: '11', value: 11 },
{ text: '12', value: 12 }
];
items.forEach((item) => {
const option = document.createElement('ion-picker-column-option');
option.value = item.value;
option.textContent = item.text;
column.appendChild(option);
});
</script>
`,
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',
Expand Down

0 comments on commit d0b6244

Please sign in to comment.