docs(edit-content): spec for #37465 Date/Time field width, clearing and picker footer - #37542
Merged
adrianjm-dotCMS merged 10 commits intoSep 18, 2026
Merged
Conversation
…nd picker footer PR 1 of 2. Spec only — no implementation. Two facts verified against the code rather than taken from the issue shape the priorities: - The timezone renders today only when the field also carries a hint, because the field's footer row is drawn only when there is a hint or a required error. On a Date-and-time field with a timezone and no hint it is never shown at all, so moving it into the picker is a coverage fix, not a relocation. - The current Today action reads the browser's clock, so it can set the wrong calendar day when the server is in another timezone. FR-013 and SC-005 make correcting that explicit. Two open decisions were resolved with the developer before writing and are recorded as Assumptions 2 and 3: the hint returns to the field footer (FR-009), and Today/Now keeps the existing per-type picker-close behaviour (FR-014a). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
33 tasks
This comment was marked as resolved.
This comment was marked as resolved.
…ooter Scope addition beyond the issue, decided in spec review: the picker footer gains a second button that confirms the selection on Date-and-time and Time-only fields. On those two types the picker no longer writes to the field as the author clicks — it holds a pending selection that reaches the field only on Accept, and dismissing without Accept discards it. Date-only is deliberately excluded: applying the gate there would turn the most common case, setting a plain date, from one interaction into two. It keeps today's behaviour exactly and shows no Accept button. Three consequences were inferred rather than stated and are flagged for a reviewer: Accept styled as the primary action, Accept always enabled, and the gate applying only to picker selections and not to typed input. FR-016a records that #37465's own acceptance criteria now contradict the spec on three points and must be corrected. Like FR-016 it records the obligation only — editing the issue needs developer approval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…picker footer" This reverts commit e39d460. The Accept button was a scope addition proposed during spec review. The developer decided to build what the issue's acceptance criteria already specify instead, so the spec returns to the single-button footer with the value applied on selection. That also drops FR-016a, which existed only to record that the issue's criteria contradicted the Accept gate. With the gate gone there is no contradiction and #37465 needs no correction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FR-016 said "the criterion", following the singular wording of #37465's own body. Checking #37464's description found four passages written around the timezone line under the input, not one: the timezone-versus-hint collision criterion, the criterion for a timezone with no hint, the test asserting the collision case, and the refinement-table row deciding it. FR-009 removes that line, so all four describe an element that will no longer be on the screen. FR-016 now lists each with its location in the issue and requires them to be withdrawn together — dropping only the collision criterion would leave the other three pointing at the same absent element. Still bookkeeping only: it edits a different issue, after merge, and needs developer approval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r review Review on PR #37542 found three places where the spec asserted something it then never checked. None is a defect; all three would have let PR 2 be judged against a contract with holes in it. - FR-014a said the picker stays open on Date-and-time and Time-only and closes on Date-only, and no acceptance scenario or success criterion covered it. That is the one place the picker's lifecycle is deliberately non-uniform, so it is also the likeliest thing to regress unnoticed while the footer is rebuilt. Added US3 scenarios 9-11, one per type, and SC-006a. - Assumption 6 pinned "current time" to second precision, but FR-012 and FR-013 said only "the current time" and nothing tested it — an assumption no implementation could be measured against. The precision now lives in FR-012 with SC-005a to check it, and the assumption points at both. - FR-009 read "removed for all three field types", which asks for the removal of an element a Date-only field never rendered. Reworded to name the types that render one today, matching the verified-behaviour table and US2. Both new criteria were checked against the implementation before being written: the open/closed assertions already exist in the child spec, and second precision holds in the unit test and in the verified save/reopen round-trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
adrianjm-dotCMS
marked this pull request as ready for review
September 15, 2026 16:20
adrianjm-dotCMS
added a commit
that referenced
this pull request
Sep 15, 2026
…ng labels (#37465) Review on PR #37542 surfaced a real bug: getCurrentServerTime(null) returned UTC clock components reinterpreted as local, while every other timezone-less path in this field (convertServerTimeToUtc, convertUtcToServerTime) treats a missing zone as "use the browser's local clock as-is". The mismatch meant Today/Now displayed one time and stored another, off by the browser's UTC offset — permanently, if the timezone request fails outright rather than merely arriving late. Falls back to the browser's own local clock instead, matching the convention already used elsewhere. No change to the conversion helpers themselves, so FR-017's storage guarantees are untouched. Also: the footer's timezone text now truncates (min-w-0 + truncate, with a title attribute carrying the full label) rather than being able to push the Today/Now button out of the overlay on a very long zone name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… after review Second round of review on PR #37542. One finding was a real bug, not a spec gap: - FR-013 didn't say what Today/Now does when the system timezone hasn't loaded yet. Traced it into calendar-field.util.ts: getCurrentServerTime(null) returned UTC clock components read as local time, while every other timezone-less path in this field (convertServerTimeToUtc, convertUtcToServerTime) treats a missing zone as "use local as-is". The mismatch meant a value set with no timezone loaded displayed one time and stored another, off by the browser's UTC offset — permanently, if the timezone request fails outright. FR-013 now states the fallback; SC-006c checks it. Fixed in the implementation on the impl branch, not just documented here. - The "very long timezone labels" edge case had no FR/SC backing it. Added FR-008b and pointed the edge case at it. - The footer action had no keyboard-reachability requirement, unlike the clear control's FR-007a. Added FR-015b, symmetric wording, plus SC-006b. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
adrianjm-dotCMS
added this pull request to stack #37557
September 15, 2026 18:39
…#37465 Third round of review on PR #37542. Two findings: FR-013's browser-clock fallback (added last round to fix the display/storage offset bug) meant that if the system timezone genuinely never resolves — its load request has no retry, confirmed in libs/global-store's withSystem — the shortcut can set the browser's day instead of the server's. SC-005 demanded the server's day in 100% of attempts and SC-006c claimed no offset "whether or not the timezone has loaded": neither was satisfiable in that state as worded, so an implementer couldn't tell whether the fallback was required behaviour or a defect. Resolved with the developer, asked directly rather than assumed: keep the fallback. A button that goes permanently dead on a timezone-service failure is worse than one that stays internally consistent (display matches storage) in a case the field already tolerated before this feature — the fallback is exactly the field's pre-#37465 behaviour, narrowed to the one case where the server's zone is genuinely unknown. SC-005 now scopes to "timezone already resolved"; SC-006c now claims only display/storage agreement, not day accuracy; FR-013 states the scoping explicitly; a new Assumption 9 records the decision and why. No code change needed — calendar-field.util.ts already matched this before the spec did. Also fixed a broken in-doc link: [FR-016](#fr-016) doesn't resolve because FR-016 is a bold list item, not a heading. Replaced with plain text, matching the "see FR-008b" convention already used elsewhere in the spec. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l gaps (#37465) Audited the spec for anything reading as an unresolved decision or a deferred question, per the developer's request — not the review comments, the spec's own language. Two kinds of problem, both closed: Three Assumptions (2, 3, 9) said "confirmed with the developer" / "Decided with the developer". None was missing content — each already stated the full rule and its rationale. The attribution was the problem, not the decision: it reads as an unresolved negotiation even though the paragraph right after it settles the question. Dropped the attribution in all three; the rule now stands on its own like every other Assumption. Two items were genuinely unresolved, caught by external review of PR #37542 rather than by this pass: - SC-005a still claimed the shortcut "matches the server's clock" unconditionally, after SC-005/SC-006c had already been scoped to the resolved-timezone case in the prior commit. Reworded to claim precision only; US3 scenarios 4-7 got the same resolved-timezone caveat. - FR-009 never stated what governs the field footer's visibility once the timezone line is gone. Added FR-009a. Verified against text-field's actual template before writing it, not assumed: the sibling pattern is mutually exclusive (error or hint, never both), while this field's footer stacks them (error first, hint below) — a first draft of FR-009a claimed parity with the siblings and was wrong; caught and corrected before this commit, with the divergence pointed at Out of scope. FR-016's "requires developer approval" was checked and left alone: it gates editing #37464 after merge, correctly scoped as bookkeeping. A sweep for approval/sign-off/pending/TBD/left-to/open-item found nothing else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Tightens the spec so every requirement states what is built, with no assumptions, deferred decisions or conditional branches left for a reader to resolve. - Drops the Assumptions section. Its substantive content already lived in the FRs; the one rule that didn't (no per-field configuration) becomes FR-020. - Today/Now always resolves through the existing getCurrentServerTime / convertServerTimeToUtc path, as the issue's acceptance criterion requires. The browser-clock fallback is gone from FR-013, and with it SC-006c and the timezone-resolved caveats on SC-005, SC-005a and the US3 scenarios. - Today/Now leaves the picker open on all three field types, the same as selecting a day or time (hideOnDateTimeSelect is false for all three, per #36156). FR-014a previously claimed Date-only closes "as a completed date selection does today", which is not what the field does. - Width is defined as 100% of the column in single- and multi-column layouts; the undefined "narrow viewport" and "supported viewport width" wording is removed from FR-004, SC-002 and US4. - The issue's read-only clause is noted as not valid in FR-006: these fields have no read-only state. - Removes the timezone-unavailable scenario, the late-arrival and long label edge cases, FR-008b, FR-009a and FR-016 with its section, and the "Relationship to #37464" section. - Reframes the timezone change as what it is: a relocation of an existing label. Conversion already worked. User Story 2 drops from P1 to P2; the Today shortcut remains the one timezone-related defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nicobytes
reviewed
Sep 17, 2026
nicobytes
left a comment
Member
There was a problem hiding this comment.
Reviewed the spec against the current calendar-field code on main. The requirements read well and are mostly testable; leaving a few questions where I think the wording would let PR 2 be built two different ways.
rjvelazco
previously approved these changes
Sep 17, 2026
Two were behavioural and verified against the implementation first; the code changes follow separately. - FR-017 (was FR-018) only covered the in-session case. handleChangeValue receives null both when a field never held a value and when the author cleared it and saved, and re-applies the default in both — so on a field with a `now` or fixed default, clearing does not survive a reload, which contradicts US1's own independent test. The two states must now be distinguished; US1 gains scenario 7 and SC-001a measures it. Worth noting for whoever implements it: the defect predates this feature. Only the expire-date field could be cleared before, so the path was unreachable — FR-005 made the clear control universal and with it this code path. - FR-008b is new. The timezone label carries role="status" and aria-live="polite" under the input today; the rebuilt footer renders a plain span with neither. The requirement states the outcome — a screen-reader user must learn the timezone while choosing a value — and leaves the mechanism open, because an overlay changes when and how it should be announced. SC-003a measures it. The rest were wording: - FR-009a settles which wins when a required field is invalid and carries a hint: both, error first. That is what the implementation already does. - FR-013 overstated "never from the browser's clock". It now separates the zone (corrected) from the instant (still `new Date()`; correcting clock skew needs a server time source that does not exist and is out of scope). - SC-005a is scoped to Date-and-time and Time — Date-only stores UTC midnight, so it carries no second to resolve. SC-005 had the same flaw one line above and is scoped the same way. - FR-017..020 renumbered to FR-016..019, closing the gap left by the retired FR-016, with the one cross-reference updated. - Out of scope now names who owns the hint-placement regression test: this issue for the three calendar field types, #37464 for the cross-type consolidation and the styling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adrianjm-dotCMS
added a commit
that referenced
this pull request
Sep 18, 2026
…imezone (#37465) Implements FR-017 and FR-008b, the two requirements added to the spec after nicobytes' review of PR #37542. Both were specified there and left for this branch; without them the implementation does not satisfy its own spec. FR-017 — a clear now survives save and reopen. handleChangeValue received null both when a field had never held a value and when the author cleared it and saved, and re-applied processFieldDefaultValue in both, so on a field with a `now` or fixed default the clear silently did not survive a reload. The child now takes the contentlet and gates the default on whether it already exists, keyed on `inode` — the same signal the store uses to choose between initializeExistingContent and initializeNewContent. A default belongs to content being created; on a saved contentlet an empty field is a value the author chose. Two parent tests moved with it: they asserted the default applies while passing a saved contentlet, which is exactly the combination FR-017 rules out. They now describe content being created, which is what they were really about. FR-008b — the timezone reaches assistive technology. The rebuilt footer rendered a plain span, so a screen-reader user learned the zone only by navigating into the overlay, if at all. A visually-hidden node in the field carries the same label and is wired through PrimeNG's ariaLabelledBy, which it forwards to the real <input> (primeng-datepicker.mjs:3298). Worth recording why it is not aria-describedby, which is what the requirement would suggest: PrimeNG exposes no ariaDescribedBy input, and setting the attribute on <p-datepicker> lands it on the host element rather than the input that takes focus — verified, the attribute was simply absent from the input. The footer label cannot be the target either: it lives in the overlay, so it exists only while the picker is open and the reference would dangle the rest of the time. All five new tests were confirmed to fail without their implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nicobytes
approved these changes
Sep 18, 2026
rjvelazco
approved these changes
Sep 18, 2026
adrianjm-dotCMS
deleted the
issue-37465-calendar-field-width-clear-picker-footer
branch
September 18, 2026 15:37
adrianjm-dotCMS
added a commit
that referenced
this pull request
Sep 18, 2026
…ng labels (#37465) Review on PR #37542 surfaced a real bug: getCurrentServerTime(null) returned UTC clock components reinterpreted as local, while every other timezone-less path in this field (convertServerTimeToUtc, convertUtcToServerTime) treats a missing zone as "use the browser's local clock as-is". The mismatch meant Today/Now displayed one time and stored another, off by the browser's UTC offset — permanently, if the timezone request fails outright rather than merely arriving late. Falls back to the browser's own local clock instead, matching the convention already used elsewhere. No change to the conversion helpers themselves, so FR-017's storage guarantees are untouched. Also: the footer's timezone text now truncates (min-w-0 + truncate, with a title attribute carrying the full label) rather than being able to push the Today/Now button out of the overlay on a very long zone name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
adrianjm-dotCMS
added a commit
that referenced
this pull request
Sep 18, 2026
…imezone (#37465) Implements FR-017 and FR-008b, the two requirements added to the spec after nicobytes' review of PR #37542. Both were specified there and left for this branch; without them the implementation does not satisfy its own spec. FR-017 — a clear now survives save and reopen. handleChangeValue received null both when a field had never held a value and when the author cleared it and saved, and re-applied processFieldDefaultValue in both, so on a field with a `now` or fixed default the clear silently did not survive a reload. The child now takes the contentlet and gates the default on whether it already exists, keyed on `inode` — the same signal the store uses to choose between initializeExistingContent and initializeNewContent. A default belongs to content being created; on a saved contentlet an empty field is a value the author chose. Two parent tests moved with it: they asserted the default applies while passing a saved contentlet, which is exactly the combination FR-017 rules out. They now describe content being created, which is what they were really about. FR-008b — the timezone reaches assistive technology. The rebuilt footer rendered a plain span, so a screen-reader user learned the zone only by navigating into the overlay, if at all. A visually-hidden node in the field carries the same label and is wired through PrimeNG's ariaLabelledBy, which it forwards to the real <input> (primeng-datepicker.mjs:3298). Worth recording why it is not aria-describedby, which is what the requirement would suggest: PrimeNG exposes no ariaDescribedBy input, and setting the attribute on <p-datepicker> lands it on the host element rather than the input that takes focus — verified, the attribute was simply absent from the input. The footer label cannot be the target either: it lives in the overlay, so it exists only while the picker is open and the reference would dangle the rest of the time. All five new tests were confirmed to fail without their implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
PR 1 of 2 — spec only
This PR carries
spec.mdalone. No implementation, no tests, no product code.Per the Spec-Kit flow,
implementation starts only once another dev approves this spec — approval, not merge:
PR 2 branches off this branch and is opened without waiting for this one to land.
Related to #37465
What the feature is
The three temporal field types in the new Edit Contentlet — Date, Time and
Date and time — are the only fields that look and behave differently from their neighbours.
They sit narrower than their column, they cannot be emptied once a value is set, and the picker
overlay's footer offers a Clear action the author does not need there while hiding the one thing
they do need: which timezone the value is interpreted in.
Four independently shippable user stories: P1 clearing on the field → P2 timezone in the
picker footer → P2 a server-accurate Today / Now → P3 full-column width.
25 functional requirements, 11 success criteria.
Two things the code said that the issue did not
Both are worth a reviewer's attention, because the issue describes neither:
is drawn only when there is a hint or a required error, so on a Date-and-time field with a
timezone and no hint the timezone is never shown at all — and where it does render, it displaces
the hint into the label tooltip. So User Story 2 is not only a relocation: it also closes a
coverage gap, which is what SC-003 measures ("up from only those fields that happen to carry a
hint today").
wrong calendar day when the server is in another timezone. FR-013 and SC-005 make correcting
that explicit instead of leaving it as a side effect of rebuilding the footer.
Two decisions the issue left open
Both were resolved before writing rather than carried as
[NEEDS CLARIFICATION], and each isstated directly by the requirement that owns it:
Overlap with #37464
#37464 (Standardize field hint and required-error presentation) is open with no spec and no
branch. This spec leaves hint and required-error presentation to it (see Out of scope), but FR-009
removes the under-input timezone line, which four passages in #37464's description are written
around — two acceptance criteria, one test criterion, and a row in its refinement table.
Whichever issue lands second should withdraw those from the other. That is cross-issue bookkeeping,
not a product requirement, so it is tracked here rather than as an FR in the spec — and editing
another issue's description needs the developer's approval before anyone acts on it.
Explicitly out of scope
Standardizing hint and required-error presentation across all field types (owned by #37464); moving
the timezone under the label; any change to how values are stored or converted between the server
timezone and UTC; the legacy Edit Contentlet screen; the picker's header, day grid and time spinner;
date/time fields rendered anywhere outside the new Edit Contentlet form.
Reviewing this
The spec is the contract PR 2 will be judged against, so the useful questions are about the
requirements, not the code: are FR-001…FR-019 testable as written, is the out-of-scope boundary
where you would draw it, and are the two decisions above the calls you would have made.
checklists/requirements.mdis gitignored per the repo's spec-folder commit policy, so it is not inthis diff.
🤖 Generated with Claude Code
This PR fixes: #37465