feat: Allow date format preference in user interface settings#29321
Draft
webalexeu wants to merge 1 commit intoevcc-io:masterfrom
Draft
feat: Allow date format preference in user interface settings#29321webalexeu wants to merge 1 commit intoevcc-io:masterfrom
webalexeu wants to merge 1 commit intoevcc-io:masterfrom
Conversation
Contributor
Author
|
Proposed improvement for #21910 |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
formatNumericDate, theymdbranch usesIntl.DateTimeFormatwith only{ month: 'numeric', day: 'numeric' }, so the year is never included; if you intendYYYY-MM-DDas per the comment, you likely need to construct that explicitly (similar toformatDayMonth) instead of relying on the en-CA locale. - The
dateFormatsetting is treated as a free-formstringin multiple places; consider introducing a shared union type or enum (e.g.'' | 'dmy' | 'mdy' | 'ymd') to constrain and validate values acrosssettings.ts,units.ts, and the formatter mixin.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `formatNumericDate`, the `ymd` branch uses `Intl.DateTimeFormat` with only `{ month: 'numeric', day: 'numeric' }`, so the year is never included; if you intend `YYYY-MM-DD` as per the comment, you likely need to construct that explicitly (similar to `formatDayMonth`) instead of relying on the en-CA locale.
- The `dateFormat` setting is treated as a free-form `string` in multiple places; consider introducing a shared union type or enum (e.g. `'' | 'dmy' | 'mdy' | 'ymd'`) to constrain and validate values across `settings.ts`, `units.ts`, and the formatter mixin.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Adds a date format dropdown in User Interface settings with four options:
- Auto: locale-native ordering (existing behavior, unchanged)
- DD/MM/YYYY: day-first ordering
- MM/DD/YYYY: month-first ordering
- YYYY-MM-DD: ISO ordering
Month and weekday names always follow the UI locale regardless of the
chosen ordering. The setting is persisted to localStorage via the
DateFormat union type ('' | 'dmy' | 'mdy' | 'ymd') shared across
settings, units and formatter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
0a1371e to
6aebf76
Compare
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.
Adds a date format dropdown in User Interface settings with four options:
Month and weekday names always follow the UI locale regardless of the chosen ordering. The setting is persisted to localStorage.
fixes #21910