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): support typing time values in a 24-hour format #30147

Merged
merged 10 commits into from
Mar 26, 2025

Conversation

vunizhona
Copy link
Contributor

  • 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.

Issue number: resolves #28877


What is the current behavior?

In the ion-datetime component, when typing 2000 in the keyboard the resulted time value is 02:00 (in 24-hour format)

Examples: https://forum.ionicframework.com/t/ion-datetime-disable-opening-keyboard/224558/6?u=dennisskylegs

What is the new behavior?

In the ion-datetime component, when typing 2000 in the keyboard the resulted time value is 20:00 (in 24-hour format)

Does this introduce a breaking change?

  • Yes
  • No

- 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
@vunizhona vunizhona requested a review from a team as a code owner January 22, 2025 13:10
Copy link

vercel bot commented Jan 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 26, 2025 4:45pm

@vunizhona
Copy link
Contributor Author

We need this update so our project can work properly. Are there any timeframes when this can be looked at? @brandyscarney

@vunizhona
Copy link
Contributor Author

We need this update so our project can work properly. Are there any timeframes when this can be looked at? @brandyscarney

There is one slightly annoying case with inputting hours within the 1-9 range. For 3-9, these hour values can be put by simply inputting the single-digit value. but for the 1 and 2, it’s impossible since the current logic puts it into the 1st place. The problem could be solved by allowing inputting single-digit values in double-digit representation with 0 in the first place (many pickers allow that logic)

We should consider how to resolve that case as well.

…se the stale time, preventing hours from updating correctly after the first time they got updated
@ShaneK ShaneK changed the title fix(datetime): allow accurate typing of time values in 24-hour format fix(datetime): allow typing of time values in 24-hour format Mar 25, 2025
@ShaneK ShaneK changed the title fix(datetime): allow typing of time values in 24-hour format fix(datetime): support typing time values in a 24-hour format Mar 25, 2025
@ShaneK
Copy link
Member

ShaneK commented Mar 25, 2025

Hello! Thank you so much for your PR!
I've made a few changes to generally make things more flexible:

  • Remove repetitive code, reliance on hard coded starting values
  • Added support for continuous typing - e.g. if you typed 0222 it would select 2:22, if you kept typing so you had input 02221010 it would select 10:10
  • Fixing a bug that revealed itself in the datetime time view if you typed over time input like that, which occurred because it was updating itself with stale state data

There's a dev build for these changes here: 8.5.2-dev.11742937752.14b430a3

We will hopefully get this reviewed and merged soon and then it'll be in the next patch version.

@ShaneK ShaneK requested review from thetaPC and removed request for ShaneK March 25, 2025 23:16
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Looks great, thank you both for all of your work on this! 🚀

@ShaneK ShaneK added this pull request to the merge queue Mar 26, 2025
@ShaneK
Copy link
Member

ShaneK commented Mar 26, 2025

Congrats on your first PR to Ionic Framework, @vunizhona! This will be merged to main as soon as it's done going through its final checks, then it'll be out with our next patch, which should happen very soon!

Thank you for your work on resolving this issue 💪

Merged via the queue into ionic-team:main with commit ac6e6a0 Mar 26, 2025
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: datetime, typing 20:00 selects 02:00
4 participants