NMS-20280: Time support for OnmsDatePicker, and TimeControls fixes - #8835
Open
synqotik wants to merge 2 commits into
Open
NMS-20280: Time support for OnmsDatePicker, and TimeControls fixes#8835synqotik wants to merge 2 commits into
synqotik wants to merge 2 commits into
Conversation
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).
synqotik
requested review from
cgorantla,
joseanesONMS and
marshallmassengill
September 1, 2026 23:35
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.
NMS-20280: Time support for
OnmsDatePicker, andTimeControlsfixesAdds time-of-day support to the
OnmsDatePickerseam wrapper, then uses it toreplace the fake time picker in
TimeControls— which turned out to have asilent 13-hour bug — and cleans up several long-standing annoyances in that
component.
OnmsDatePicker(onms-uicomponent seam layer)You can use
timeOnlyto show only time controls;showTimeto include time controls under the date picker; or just display the date picker.Now declares the following props.
showTimebooleanfalsetimeOnlybooleanfalseshowSecondsbooleanfalsehourFormatOnmsHourFormat('12' | '24')'24'stepHour/stepMinute/stepSecondnumber1placeholderstringminDate/maxDateDate | nullinputIdstringPlus new
show/hideemits reporting overlay visibility.Notes for reviewers:
OnmsTimePickeris needed.timeOnlyworks on its own — PrimeVuegates the time panel on
showTime || timeOnlyand the date panel on!timeOnly.site's rendering. These are not "notable deviations" in the README sense.
hourFormatis typed as a new ownedOnmsHourFormatinpackages/onms-ui/src/types.ts, not PrimeVue'sHintedString<'12' | '24'>—seam rule 4 (no PrimeVue types in a public signature).
minDate/maxDateacceptDate | nulland collapsenulltoundefinedbefore reaching PrimeVue, which types them
Date | undefinedand treats anynon-
undefinedvalue as a live bound. This keeps a?? undefinedout ofevery call site that holds its bounds in a nullable ref.
inputIdlands on the rendered<input>, not the wrapper, so aFormFieldlabel's
forcan reach the real control — matching whatOnmsSelect,OnmsInputNumberand the other input wrappers already declare.which seam rule 2 classes as unsupported. Declaring them makes the API real.
TimeControlsMoved to
components/Common, along withutils/timeRangeOptions.ts—AdhocGraphBuilder.vueandadhocUrlState.tswere already importing that outof
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, andapplyCustomTimeadded the select's value to the picked date. The selectdefaulted to
1 PM, so unless the user touched both selects, every custom rangecame out 13 hours off. The test that caught it:
One
showTimepicker per range end replaces the pair. Ranges are nowminute-precise instead of whole-hour.
HOUR_OPTIONSbecomes dead and isremoved 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/:maxDatecross-wiring. PrimeVue does not enforce those strictly enough to be a correctness guarantee. The Apply button is disabled unlessend > start, the End field explains why, andapplyCustomTimere-checks rather than trusting the button's disabled state.Labels and accessibility
labelprop renders a bold label beside the trigger button.The Resource Graphs page gains the "Time Range:" label it was missing, and
AdhocChartToolbardrops its local<span>+ CSS in favor of the prop."<label id> <own id>", so its accessible nameis "Time Range: Last day" — label and current selection.
FormFields pass aformatching each picker'sinputId,so those labels are associated with their inputs for the first time.
Layout
.wrapper's fixedheight: 70px(a 70px box around a 35px button) sothe label and button text align on a baseline.
variant="ghost", with1embetween the Start,End and Apply blocks.
External References