Repository navigation
feat(web): offer common date/time format presets, with a custom fallback - #463
Conversation
Closes #452. The Date format / Time format fields on Farm settings were plain free-text inputs accepting any string up to 32 chars — server-side validation (BeAUsableFormat) only rejects a string that literally throws as a .NET custom format, so something like "asdf" saved successfully and rendered as a garbled date everywhere, rather than being caught at entry. Replaces the free-text field with a dropdown of common presets (MM/dd/yyyy, dd/MM/yyyy, yyyy-MM-dd for date; h:mm tt, HH:mm for time) plus a "Custom…" option that reveals the original free-text field for anything not listed — so most people never need to know format-pattern syntax, while the escape hatch (and the server's own validation of it) stays available. No backend change: this is purely about how most users arrive at a value, not what's accepted. A stored value that isn't blank and doesn't match a preset opens the dropdown directly on "Custom…" with the real value already in the revealed field, rather than silently falling back to "Follow the locale" and discarding it on the next save — handled via a separate *Custom boolean per field rather than deriving it purely from the value, since picking "Custom…" from an empty/follow-locale start doesn't change the value at all, so a purely-derived flag would immediately collapse back to "Follow the locale" and the field would never appear. Rewrote one existing test that fired a raw DOM `change` event with a whitespace value directly on what's now a <select> — that happened to still pass (jsdom lets an out-of-range value through to the state setter regardless of the UI), but no real user interaction can produce it anymore now that the control is a dropdown. Rewrote it to go through the actual new flow: select "Custom…", then type whitespace into the revealed field. Mutation-tested: broke isCustomFormat to always return false (confirmed the "opens on Custom…" test goes red — the stored custom value would otherwise open silently on "Follow the locale" and be discarded on save) and broke the preset-select branch's Custom-flag reset (confirmed the "re-hides on preset pick" test goes red — the custom field would linger after the user picked a preset, sending stale typed text instead of the selected preset). Both reverted after confirming.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c16b739433
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ssary codex review of PR #463: farmSettingsIntro and glossaryFarmSettingsDef already said date/time formats are optional (still true, so not wrong), but neither explained the new preset-dropdown + Custom workflow this PR introduced — a user-visible behavior change with no way to discover it from the Help page. Adds a dedicated Help bullet (farmSettingsDateTimeFormat, matching the existing per-topic pattern — farmSettingsTimezone/Currency/Logo/ CountingUnit each get their own <li>) and extends the Farm settings glossary definition with the one clause. es/tl machine-drafted, matching every other string in these catalogs pending the native-speaker review tracked in #182 (Phase 1.5).
|
@codex address that feedback |
Summary
Testing
|
codex follow-up on PR #463 (its own commit 99617b4 never made it to this branch — same sandbox git-remote limitation as before — applied the equivalent fix myself): the Help bullet said "an unusable value is refused rather than saved," but BeAUsableFormat only rejects a string that literally throws as a .NET format — something like "asdf" passes and renders as garbage, which is exactly the OTHER half of #452 (tightening that validator) this PR deliberately left as a follow-up. Softened the claim to match what the server actually guarantees today.
|
Good catch, and a real one: "an unusable value is refused rather than saved" overstated what Your |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 142d52b4bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex review of PR #463: the previous fix told admins to "check it renders the way you expect after saving" — impossible advice. specs/product/GLOSSARY.md:482-489 already documents that locale, unit system and the date/time format overrides are "stored and validated, and nothing renders through them yet" (money still prints as 12.34 USD, dates as ISO) — confirmed by a repo-wide search finding no SPA consumer of dateFormatOverride/timeFormatOverride outside the settings form itself. Rewrote the Help/es/tl text to match: the presets are safe choices, a custom value is only checked well enough to save (not proven correct), and nothing displays through this setting yet, so a broken custom value wouldn't be visible today either.
|
@codex address that feedback Generated by Claude Code |
Summary
Testing
|
Closes #452.
Problem
The Date format / Time format fields on Farm settings were plain free-text inputs accepting any string up to 32 chars. Server-side validation (
UpdateFarmSettingsValidator.BeAUsableFormat) only rejects a string that literally throws when used as a .NET custom format — .NET's custom date/time formatter treats most unrecognized letters as literal text rather than erroring, so something like"asdf"saves successfully and then renders as a garbled/nonsensical date everywhere, rather than being caught at entry.Fix (scoped down from #452's full ask — SPA only)
Replaces the free-text field with a dropdown of common presets:
MM/dd/yyyy,dd/MM/yyyy,yyyy-MM-ddh:mm tt,HH:mm...plus a "Custom…" option that reveals the original free-text field for anything not listed — most people never need to know format-pattern syntax, while the escape hatch (and the server's own validation of it) stays fully available. No backend change: this is purely about how most users arrive at a value, not what's accepted — tightening
BeAUsableFormatitself is the other, separate half of #452, left for a follow-up.A stored value that isn't blank and doesn't match a preset opens the dropdown directly on "Custom…" with the real value already in the revealed field, rather than silently falling back to "Follow the locale" and discarding it on the next save.
A design note worth flagging
The "is this custom?" state is tracked as its own boolean per field, not derived purely from the stored value. Deriving it would break the empty-value case: picking "Custom…" from an empty ("Follow the locale") start doesn't change the underlying value at all, so a purely-derived flag would immediately collapse back to "Follow the locale" and the free-text field would never actually appear — a real functional bug I caught by tracing through the interaction before writing it, not by testing it in afterward.
Testing
npm run typecheck/npm run i18n:scan(COUNT: 3, unchanged baseline) /npm test(1416/1416, +4 new) all green.changeevent with a whitespace value directly on what's now a<select>. It happened to still pass (jsdom lets an out-of-range value through to the state setter regardless of the UI), but no real user interaction can produce that anymore — rewrote it to go through the actual new flow (select "Custom…", then type whitespace into the revealed field).isCustomFormatto always returnfalse— confirmed the "opens on Custom…" test goes red (a stored custom value would otherwise silently open on "Follow the locale" and get discarded on the next save).Docs
Checked
specs/product/GLOSSARY.mdand the Help page's Farm settings copy — both already describe the concept ("leave them blank and the locale decides") without referencing the old free-text mechanism specifically, so neither needed updating.