Skip to content

NMS-20280: Time support for OnmsDatePicker, and TimeControls fixes - #8835

Open
synqotik wants to merge 2 commits into
foundation-2026from
jira/NMS-20280-onms-timepicker
Open

NMS-20280: Time support for OnmsDatePicker, and TimeControls fixes#8835
synqotik wants to merge 2 commits into
foundation-2026from
jira/NMS-20280-onms-timepicker

Conversation

@synqotik

@synqotik synqotik commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

NMS-20280: Time support for OnmsDatePicker, and TimeControls fixes

Adds time-of-day support to the OnmsDatePicker seam wrapper, then uses it to
replace the fake time picker in TimeControls — which turned out to have a
silent 13-hour bug — and cleans up several long-standing annoyances in that
component.

OnmsDatePicker (onms-ui component seam layer)

You can use timeOnly to show only time controls; showTime to include time controls under the date picker; or just display the date picker.

Now declares the following props.

prop type default
showTime boolean false
timeOnly boolean false
showSeconds boolean false
hourFormat OnmsHourFormat ('12' | '24') '24'
stepHour / stepMinute / stepSecond number 1
placeholder string
minDate / maxDate Date | null
inputId string

Plus new show / hide emits reporting overlay visibility.

Notes for reviewers:

  • No OnmsTimePicker is needed. timeOnly works on its own — PrimeVue
    gates the time panel on showTime || timeOnly and the date panel on
    !timeOnly.
  • Every default matches PrimeVue's own, so this changes no existing call
    site's rendering. These are not "notable deviations" in the README sense.
  • hourFormat is typed as a new owned OnmsHourFormat in
    packages/onms-ui/src/types.ts, not PrimeVue's HintedString<'12' | '24'>
    seam rule 4 (no PrimeVue types in a public signature).
  • minDate / maxDate accept Date | null and collapse null to undefined
    before reaching PrimeVue, which types them Date | undefined and treats any
    non-undefined value as a live bound. This keeps a ?? undefined out of
    every call site that holds its bounds in a nullable ref.
  • inputId lands on the rendered <input>, not the wrapper, so a FormField
    label's for can reach the real control — matching what OnmsSelect,
    OnmsInputNumber and the other input wrappers already declare.
  • These props were previously reachable only by Vue attribute fallthrough,
    which seam rule 2 classes as unsupported. Declaring them makes the API real.

TimeControls

Moved to components/Common, along with utils/timeRangeOptions.ts
AdhocGraphBuilder.vue and adhocUrlState.ts were already importing that out
of components/Resources/. Both moves are recorded as git renames.

Bug fix: custom ranges were silently shifted 13 hours

The old UI was a date picker plus a separate whole-hour OnmsSelect, and
applyCustomTime added the select's value to the picked date. The select
defaulted to 1 PM, so unless the user touched both selects, every custom range
came out 13 hours off. The test that caught it:

expected startTime 1788330600, got 1788283800   // Δ = 46800s = 13h

One showTime picker per range end replaces the pair. Ranges are now
minute-precise instead of whole-hour. HOUR_OPTIONS becomes dead and is
removed with its test.

Clicking the preset list while date picker open no longer closes the popover

Previously, clicking the preset list ("Last hour", …) while a date picker
overlay was open selected a preset and closed the popover. Now that click only
dismisses the picker, keeping its selection, and leaves the popover open.

Range validity

An explicit ordering check, not the :minDate / :maxDate cross-wiring. PrimeVue does not enforce those strictly enough to be a correctness guarantee. The Apply button is disabled unless
end > start, the End field explains why, and applyCustomTime re-checks rather than trusting the button's disabled state.

Labels and accessibility

  • New optional label prop renders a bold label beside the trigger button.
    The Resource Graphs page gains the "Time Range:" label it was missing, and
    AdhocChartToolbar drops its local <span> + CSS in favor of the prop.
  • The trigger button is named by "<label id> <own id>", so its accessible name
    is "Time Range: Last day" — label and current selection.
  • The Start and End FormFields pass a for matching each picker's inputId,
    so those labels are associated with their inputs for the first time.

Layout

  • Dropped .wrapper's fixed height: 70px (a 70px box around a 35px button) so
    the label and button text align on a baseline.
  • "Apply custom time" is now variant="ghost", with 1em between the Start,
    End and Apply blocks.

External References

OnmsDatePicker (seam layer):
- Declare the time props showTime / timeOnly / hourFormat / showSeconds and
  stepHour / stepMinute / stepSecond, plus placeholder / minDate / maxDate.
  PrimeVue supports seconds natively (showSeconds, stepSecond), so no
  fallback is needed, and timeOnly hides the calendar on its own -- hence no
  separate OnmsTimePicker. Every default matches installed primevue@4.5.5's
  own, so no existing consumer's rendering changes. Passing these by
  attribute fallthrough happens to reach PrimeVue, but seam rule 2 classes
  that as unsupported.
- hourFormat is typed as a new owned OnmsHourFormat ('12' | '24') rather than
  PrimeVue's HintedString, per seam rule 4.
- minDate/maxDate accept Date | null and collapse null to undefined, which
  PrimeVue requires; consumers keep their bounds in nullable refs.
- Emit show / hide so a consumer can observe overlay visibility without
  reaching into PrimeVue internals.

TimeControls:
- Moved to components/Common, together with utils/timeRangeOptions.ts (which
  AdhocGraphs already imported out of Resources). Consumers updated.
- Replace the date-picker-plus-hour-select pair with one showTime picker per
  range end. This also fixes a bug: the hour select defaulted to 1 PM and was
  ADDED to the picked date, so a custom range was silently shifted 13 hours
  unless both selects were touched. Ranges are now minute-precise. The dead
  HOUR_OPTIONS and its test go away.
- A click on the preset list while a picker overlay is open now only dismisses
  that picker, keeping its selection and leaving the popover open, instead of
  selecting a preset. PrimeVue dismisses the overlay from a document-level
  mousedown listener, so the state is latched on mousedown over the list -- a
  check at click time always reads "closed".
- Cross-wire minDate/maxDate between the two pickers so a range cannot invert.
- New optional `label` prop renders a bold label beside the trigger button and
  names it with aria-labelledby. AdhocChartToolbar drops its local span and
  CSS for it; the Resource Graphs page gains the label it was missing. Doing
  this in the component is what the AdhocChartToolbar comment asked for: the
  attribute applied from outside lands on a plain root div and is inert.
- Drop .wrapper's fixed 70px height (a 70px box around a 35px button) so the
  label and button text share a baseline; apply is now variant="ghost", with
  1em between the Start, End and Apply blocks.

Tests: OnmsDatePicker contract tests for every new prop and emit, and a new
TimeControls suite covering the custom-range arithmetic, the mousedown-latched
preset suppression, the range bounds and the label wiring.
Accessible name of the range button (finding 1): aria-labelledby REPLACES an
element's content as its accessible name, so pointing it at the label span
alone renamed the button from "Last day" to "Time Range:" -- dropping the
selected range, the only changing information the button carries. The button
now carries its own id and is labelled by "<label id> <own id>", giving
"Time Range: Last day".

Range validity (finding 2): the :minDate/:maxDate cross-wiring is a UI
affordance, not a correctness guarantee. PrimeVue's isSelectable() compares
year/month/day only, and the typed-input path (isValidSelection, with
manualInput defaulting to true) goes through it -- so typing a later time on
the boundary day into Start is accepted unclamped and inverts the range, which
then reads as a negative difference and mislabels the window as "minutes". A
calendar click does clamp at full granularity, but clamping Start to maxDate
lands it exactly on End, which is a zero-width window. Both slipped past the
old null-only check. Apply is now disabled unless end > start, the End field
explains why, and applyCustomTime re-checks rather than trusting the button
state.

Label association (finding 4): OnmsDatePicker gains an inputId prop, which
PrimeVue puts on the rendered <input> (a plain id would fall through to the
wrapper, out of a label's reach), matching what OnmsSelect / OnmsInputNumber
and the other input wrappers already declare. The Start and End FormFields
pass a matching `for`, so their labels are associated for the first time.

Overlay-visibility window (finding 3): confirmed -- PrimeVue emits `show` from
the overlay's onEnter hook but binds its document dismisser in onAfterEnter, so
during the enter transition a picker reports itself open while nothing will
dismiss it. Left as-is and documented in the code: a press in that window makes
the preset click a no-op rather than a wrong action, and the next click behaves
normally. Suppressing only when the picker actually closed would restore the
original bug inside that same window, and PrimeVue exposes no way to dismiss
the overlay ourselves (onBlur does not close it; overlayVisible is internal).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant