Repository navigation
fix(date-field): show PM times correctly in 12-hour display - #586
Merged
Merged
Conversation
fieldsFromValue copied the 0-23 hour straight into the segments and never set dayPeriod, while composeValue reads 12-hour segments as `hour % 12 + (pm ? 12 : 0)`. 18:00 showed as "18:00 AM" and any edit saved it as 06:xx. Split the hour into 1-12 + dayPeriod in 12-hour mode, and re-encode stored segments when the hour cycle flips on a mounted field (hourCycle prop or locale change). Refs tailor-inc/platform-planning#2039 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
/review |
Contributor
|
✅ Code Review completed successfully! Code review complete for PR #586: reviewed changed package implementation under packages/core, checked prior PR review threads/comments, and found no actionable High/Medium/Low package issues to flag. No GitHub write was needed because there were no inline findings to post.
|
Contributor
Code Metrics Report
Details | | main (529b3ac) | #586 (1bc7d88) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 87.6% | 87.9% | +0.3% |
| Files | 206 | 206 | 0 |
| Lines | 6090 | 6097 | +7 |
+ | Covered | 5335 | 5364 | +29 |
- | Test Execution Time | 2m14s | 2m19s | +5s |Code coverage of files in pull request scope (89.5% → 94.6%, patch 93.3%)
Reported by octocov |
interacsean
marked this pull request as ready for review
October 8, 2026 23:34
IzumiSy
approved these changes
Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes tailor-inc/platform-planning#2039
Problem
With
granularityathour/minute/secondand a 12-hour cycle (hourCycle={12}or a 12-hour locale likeen-US),DateField/DatePicker/DateRangePickershowed afternoon values as e.g. "18:00 AM", and editing any segment emitted the value 12 hours early (06:xx).fieldsFromValue()copied the 0–23 hour straight into the segments and never setdayPeriod, whilecomposeValue()reads 12-hour segments ashour % 12 + (pm ? 12 : 0).Fix
fieldsFromValue(v, is12)stores the hour as 1–12 +dayPeriodin 12-hour mode (midnight → 12 AM, noon → 12 PM). Used for the initial value, external controlled syncs, and the anchor, so ArrowUp on an empty hour after noon no longer seeds an out-of-range18.hourCycleprop or alocalechange that flips the default), the stored segments are re-encoded during render. The composed value is unchanged, soonChangedoesn't fire. Half-typed hours convert too.granularity/hourCyclerows to the DatePicker props table.Verification
date-field.test.tsxanddate-range-picker.test.tsx. Against the old hook, 9 fail (the 00:00 minute-edit case passes there by coincidence, since 0 % 12 is AM); without the cycle-change conversion, exactly the 3 switch tests fail.06:00 PM; minute ArrowUp →18:01; toggling 12↔24 shows18:01/06:01 PM;a/pon AM/PM → 06:01 / 18:01.🤖 Generated with Claude Code