diff --git a/docs/UI_UX_DECISION_LEDGER.md b/docs/UI_UX_DECISION_LEDGER.md index dae30f3..a3f9269 100644 --- a/docs/UI_UX_DECISION_LEDGER.md +++ b/docs/UI_UX_DECISION_LEDGER.md @@ -46,6 +46,10 @@ contestability, responsibility, and traceability at the point of action. | UX-020 | Centrally exported Core components are mandatory wherever their contract covers the interaction. A custom reusable control, presentation primitive, or module-local substitute requires explicit product-owner authorization, a narrowly specific purpose, and documented rationale and scope; it must not duplicate a central component. | Accepted | Core WebUI and all module WebUIs | | UX-021 | A collection-wide create action belongs in that collection's page heading and is not duplicated in a persistent side panel. When a side panel is the creation surface, it is present for the creation view only. | Accepted | List-detail, directory, and create surfaces | | UX-022 | Use central `Card` components for logical sections, `DataGrid` for tabular row collections and their ordered actions, and `ToggleSwitch` for boolean settings. Repeatable people/contact editors use one structured row per person with name, email address, and actions; free-form address parsing is reserved for an explicitly designed bulk-import flow. | Accepted | All WebUI forms and collection editors | +| UX-023 | `FieldLabel` is the standard label/help surface for every field that is not self-explanatory. Any field rendered without it must be recorded in the omission register below, including its accessible-name source and rationale. Users may hide inline help markers through their persisted interface preference; the field label itself remains visible. | Accepted | All Core and module forms | +| UX-024 | Explicit `Discard` actions and dirty in-application navigation use the shared `UnsavedChangesProvider` dialog. A page registers save/discard behavior with `useUnsavedDraftGuard`; its Discard button calls `requestDiscard`, and route changes use `useGuardedNavigate` or `requestNavigation`. | Accepted | All create/edit surfaces | +| UX-025 | `window.alert` and the global `alert` function are prohibited. A narrowly necessary exception requires product-owner authorization and an entry in the alert exception register before implementation. | Accepted | All WebUI code | +| UX-026 | A table defines one stable ordered action set. A row-level unavailable action remains in its normal position and is disabled, preferably with `disabledReason`; structurally irrelevant actions are omitted for the entire table. Empty rows reserve the same slots so their Add action stays in the normal left-most action position. | Accepted | All structured tables | ## Confirmed Implementation Decisions @@ -191,9 +195,10 @@ explicitly retains the exception. Decision: Scheduling requests provide a concrete reference application of the universal placement and component rules. -- The `Scheduling requests` header owns one `Add` action. -- Its left panel is used for the creation view only, not as a permanent second - creation affordance. +- The persistent left panel stacks `My scheduling requests` and `Scheduling + requests for me`; it is list context, not a second creation affordance. +- The left panel's `Scheduling requests` header owns one `Add` action. It opens + the shared view/create/edit surface in the right main panel. - Basic information, Calendar integration, candidate slots, and participants use the central `Card` component as four logical sections. - Candidate slots and participants use the central `DataGrid`, including its @@ -207,6 +212,50 @@ universal placement and component rules. Equivalent list/create/edit surfaces use the same underlying rules. These are not Scheduling-local component variants. +### DUE-011: Field Help, Discard, And Table Action Contracts + +Decision: the central components own these interactions; modules compose them +instead of reproducing their behavior. + +- `FormField` and `ToggleSwitch` already render `FieldLabel`. Direct field + compositions use `FieldLabel` explicitly when the meaning or limitation is + not self-explanatory. +- `help` content is contextual guidance, not the accessible name. The persisted + `show_inline_help_hints` user preference hides only the `InlineHelp` marker by + applying `ui-hide-help-hints` at the document root. +- A dirty editor registers once with `useUnsavedDraftGuard`. An explicit + Discard button calls `useUnsavedChanges().requestDiscard(afterResolve)`; SPA + navigation uses `useGuardedNavigate` or `requestNavigation`. Both paths show + the same shared unsaved-changes dialog. A browser tab/window unload remains a + browser-controlled confirmation because browsers do not permit a custom + modal at that boundary. +- `TableActionGroup` receives the table's stable action set. Use `disabled` and + `disabledReason` for row state; omit an action only when that action does not + belong to the table. `minimumSlots` reserves trailing positions for an empty + row. `DataGridEmptyAction` does this for the standard add/move/remove layout. +- Feedback and confirmation use `Dialog`, `ConfirmDialog`, or + `DismissibleAlert`. They never fall back to `window.alert`. + +#### FieldLabel Omission Register + +Every Core field surface that intentionally does not render `FieldLabel` is +listed here. Module repositories keep an equivalent register in their durable +UI documentation until a central cross-repository audit is available. + +| Core scope | Why `FieldLabel` is omitted | Accessible/context label source | +| --- | --- | --- | +| `PasswordField`, `ColorPickerField`, `DateField`, `TimeField`, and `DateTimeField` input internals | These are label-neutral composite primitives and are placed inside `FormField`/`FieldLabel` by the consuming form. Rendering another label inside the primitive would duplicate it. | Enclosing label; a direct consumer must pass an accessible name and record that direct composition here. | +| `ToggleSwitch` native checkbox | The shared component already renders its visible text through `FieldLabel`; the native input must not render a second label. | The enclosing native label and derived `aria-label`. | +| `FileDropZone` hidden file input | The input is an implementation detail of the labelled keyboard-operable drop target. | Drop target text and `inputLabel`/`aria-label`. | +| `AdminSelectionList` and `DataGrid` list-filter checkboxes | Each option is self-explanatory and already enclosed by its visible option label. | Enclosing native option label. | +| `EmailAddressInput` compact Name and Email fields | These two conventional fields are self-explanatory in the compact address popover; richer address guidance belongs to the enclosing field. | Visible native labels; the free-form editor also has a descriptive `aria-label`. | +| `DataGrid` page-size, filter, and inline cell editors | The surrounding column header/filter heading supplies field context; repeating a labelled help marker in every cell would add noise. | Column header, filter heading/native label, or generated cell `aria-label`. | +| Retention-policy value controls | `PolicyRow` owns the field label, help, effective value, and provenance for its control. | The containing `PolicyRow` label/help contract. | + +#### Alert Exception Register + +No `window.alert` or global `alert` exception is authorized. + ## Implementation Sequence | Phase | Scope | Output | @@ -277,6 +326,14 @@ Every new or changed admin/configuration surface should answer: - Are logical sections, tabular collections, boolean settings, and repeatable people/contact rows composed with `Card`, `DataGrid`, `ToggleSwitch`, and one structured row per person respectively? +- Does every non-self-explanatory field use `FieldLabel`, and is every omission + recorded with its rationale and accessible-name source? +- Do explicit Discard and dirty navigation use the shared unsaved-changes + registration/dialog rather than a page-local confirmation? +- Does every row retain the table's action set in the same order, disabling + unavailable actions and reserving the same empty-row slots? +- Is feedback rendered with a central dialog/alert component, with no + unauthorized `window.alert` or global `alert` call? - If automation is involved, can the user see the trigger, system actor, observed effects, and failure/manual-intervention state? - Are technical details available without being the first thing the user sees? diff --git a/webui/src/components/UnsavedChangesGuard.tsx b/webui/src/components/UnsavedChangesGuard.tsx index 7e346cd..fccd133 100644 --- a/webui/src/components/UnsavedChangesGuard.tsx +++ b/webui/src/components/UnsavedChangesGuard.tsx @@ -26,6 +26,11 @@ type UnsavedChangesContextValue = { hasUnsavedChanges: boolean; registerUnsavedChanges: (registration: UnsavedChangesRegistration | null) => () => void; requestNavigation: (action: UnsavedNavigationAction) => void; + /** + * Route an explicit Discard button through the same confirmation used for + * dirty navigation. The action runs after either saving or discarding. + */ + requestDiscard: (action: UnsavedNavigationAction) => void; }; const UnsavedChangesContext = createContext(null); @@ -72,6 +77,10 @@ export function UnsavedChangesProvider({ children }: {children: ReactNode;}) { setPendingAction(() => action); }, []); + const requestDiscard = useCallback((action: UnsavedNavigationAction) => { + requestNavigation(action); + }, [requestNavigation]); + useEffect(() => { function onBeforeUnload(event: BeforeUnloadEvent) { const active = registrationRef.current; @@ -150,8 +159,9 @@ export function UnsavedChangesProvider({ children }: {children: ReactNode;}) { const value = useMemo(() => ({ hasUnsavedChanges, registerUnsavedChanges, - requestNavigation - }), [hasUnsavedChanges, registerUnsavedChanges, requestNavigation]); + requestNavigation, + requestDiscard + }), [hasUnsavedChanges, registerUnsavedChanges, requestDiscard, requestNavigation]); return ( @@ -185,7 +195,8 @@ export function UnsavedChangesProvider({ children }: {children: ReactNode;}) { const fallbackUnsavedChangesContext: UnsavedChangesContextValue = { hasUnsavedChanges: false, registerUnsavedChanges: () => () => undefined, - requestNavigation: (action) => action() + requestNavigation: (action) => action(), + requestDiscard: (action) => action() }; export function useUnsavedChanges() { diff --git a/webui/src/components/table/DataGrid.tsx b/webui/src/components/table/DataGrid.tsx index 1ebb23e..954acb1 100644 --- a/webui/src/components/table/DataGrid.tsx +++ b/webui/src/components/table/DataGrid.tsx @@ -106,6 +106,8 @@ type DataGridProps = { export type DataGridRowActionsProps = { disabled?: boolean; removeDisabled?: boolean; + /** Set to false only when row ordering is not part of this table's action set. */ + reorderable?: boolean; onAddBelow: () => void; onRemove: () => void; onMoveUp?: () => void; @@ -858,6 +860,7 @@ export function DataGridPaginationBar({ export function DataGridRowActions({ disabled = false, removeDisabled = false, + reorderable = true, onAddBelow, onRemove, onMoveUp, @@ -879,20 +882,18 @@ export function DataGridRowActions({ disabled, onClick: onAddBelow }, - { + reorderable && { id: "move-up", label: moveUpLabel, icon: