fix(datetime): datetime locale with h23 will respect max time range

[sean-perkins]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Locales that have the hour cycle in 24 hour format, will not respect the max time range specified. This means if you set the max to 20:40, they are still able to select 21, 22, 23 for hour values.

Issue Number: #24588

What is the new behavior?

• The time picker will respect the max hour range when the locale is in a 24 hour format
• Applies the same logic for restricting minutes used here fix(datetime): time picker format with hourCycle h23 #24476

Does this introduce a breaking change?

• Yes
• No

Other information

[liamdebeasi]

Can you explain why this change works? If you are using 24 hour time, refParts.ampm should be undefined, which means convertedHour would be hour.

[sean-perkins]

The workingParts (used as refParts in that util function) has ampm defined. Likely as a result of when we call processValue on initialization of the datetime component:

...
this.workingParts = {
  ...
  ampm: hour >= 12 ? 'pm' : 'am'
}

Where as the hour cycle only takes into consideration the locale or if the developer has hard-set the hour cycle.

So this case exists when the developer hasn't specified the hour cycle manually, but the locale requires it.

[liamdebeasi]

Ok thanks. Would it make more sense to not have ampm be optional then? https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/datetime/datetime-interface.ts#L21

[sean-perkins]

Yeah, I think that makes sense for now. I'll confirm we aren't using that interface for any partial date object comparisons.

[sean-perkins]

Looks like we are in a few places:

• https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/datetime/datetime.tsx#L1182
• https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/datetime/datetime.tsx#L563
• https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/datetime/datetime.tsx#L567

I'll create a tech debt task to have more strict explicit types.

[Agular]

Applies the same logic for restricting minutes used here fix(datetime): time picker format with hourCycle h23 #24476

Was this implemented? Since I am using ionic 6.0.4 and the minutes are not updated when choosing a time with the hour less than the hour of the max time. This was recently fixed for min value.