Skip to content

feat(ui): apply the view colour and count rules across the UI - #315

Merged
novoj merged 3 commits into
devfrom
feat/ui-rules-v2
Oct 29, 2026
Merged

novoj merged 3 commits into
devfrom
feat/ui-rules-v2

Conversation

@novoj

@novoj novoj commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

What

Applies the UI rules agreed on the screenshot review page to the rest of the UI (the folder header part shipped in #314):

  1. Counts: full-width rows put the count at the right end. Block headings put it right after the heading, always rendered the same way (muted, small, no parentheses, like "Proposed matches 5"). Tabs, pills and buttons keep the count after the label.
  2. Icons neutral until their view is open. A view toggle takes its view colour and a pressed look only while open. State marks keep their colour: scope, meeting, overdue, warnings, workflow, Invitations, speaker colours. Delete/revoke icons are neutral, red only on hover/focus. Inline editors are not views: their trigger stays inactive while the editor is open.
  3. One colour per view, on its toggle while open, its title icon and links into it. Everything else stays in house styles.

Fixes marked on the review page

  • R1-8: "Stored remarks (N)" is now "Stored remarks" + count in the block-heading style.
  • R1-9: "2 · Discussion points" renders its count exactly like "1 · Task review"; Suggest points stays at the right.
  • All block-heading counts use one HeadingCount: Task review and Learn sections, profile groups, stored remarks, both agenda sections, folder task groups, a person's Recordings, billing groups.
  • R2-2 / R2-8 / R3-1, item header:
    • Share is neutral; magenta only marks an existing person share.
    • Knowledge is neutral at rest in personal and Organization items. While open it is purple (--knowledge) with a pressed look, matching the folder header.
  • R2-3: folder tree menu: Prepare an agenda and Share folder icons are neutral.
  • R2-4: API key revoke and every other red-at-rest delete/revoke icon (webhooks, templates) are now neutral, red on hover/focus. Destructive menu items too (shared dropdown variant): neutral at rest, red on hover/focus, contrast ≥ 4.5:1 in both themes (computed in a test).
  • R2-10: inline editor triggers stay visible but inactive while their editor is open (InlineEditorTrigger, same look as Add task from feat(ui): folder header action row and folder Tasks panel #314). This covers task Note, meeting note, Edit summary, Add person, Add thing and nickname. Focus returns to the trigger after Save/Cancel.

Not on the review page (easy to reverse)

These came from the audit's other clear violations; please check them:

  • Dev navigation item: no longer red at rest.
  • Sign out and the Erase… menu items: neutral at rest, red on hover/focus (shared destructive menu variant).
  • Recording Agenda row: "Agenda" first, the "x of y points discussed · n tasks" summary at the right (wraps at narrow widths).
  • Transcript chapter bar: the topic title first, the "1 / 4" position at the right end.
  • Needs review filter: "Needs review 1" with the count at the right like the channel rows (was "Needs review (1)").
  • Arrival notice overflow count: at the right end.
  • Formatted mail toggle and the access-settings person rows / archived profiles: neutral pressed look (bg-accent, aria-pressed). Access rows close on a second click.
  • Organization profile group: its Users mark uses the Organization colour.
  • Summary editor header: shows "Editing summary" next to the inactive pencil (the label lost its own pencil icon so the header has one pencil).
  • Help drawer and fact source links: the external-link glyphs are neutral.

Reviewer calls (where I overrode the implementer and the cross review)

  • Task review (widget and dialog): title icon and the small labels ("Earlier task", "Said now", match legend) are neutral. Orange there would be decoration, sitting right next to the amber "review needed" edge, and the other recording widgets have neutral title icons.
  • Disclosures (recording Agenda, "Show dropped" tasks): no view colour or aria-pressed, only aria-expanded. They reveal content in place, like the widget chevrons.
  • Task note and meeting note left borders: neutral (border-border) instead of orange/teal.
  • Tasks orange: kept only on links into Tasks (task detail backlink, agenda source-task links).

Kept as is (marked fine on the review page)

R1-2 folder chips, R1-3 Knowledge tabs, R1-5 Learn pills, R1-6 Almanac Review badge, R2-5 speaker colours, R2-6 overdue pill, R2-7 Invitations launcher, R2-9 transcript chevron, R3-2 agenda actions, R3-3 Tasks page Add task, R3-4 review counts. Also kept: filled destructive buttons, scope marks and workflow colours.

Also

  • Access settings: when switching people, a late detail response for the previous person no longer replaces or hides the current person's panel. A write's outcome (error, success, session messages, sign-in prompt) is still always reported.
  • User guide: text for the inactive editor triggers (EN/CS). Screenshots retaken only where the UI changed (17 images); capture.sh follows the new "Needs review 1" label.

Tests

  • New: src/tests/ui/ covers the rules, the shared primitives, access selection races and destructive menu contrast. Run against the pre-change components, 52 + 27 of the new cases fail.
  • Updated: existing tests that asserted the old colours, labels and counts.
  • Local runs: whole unit suite 640 files / 7,328 tests passed (Node 22) before the last two small fixes. The touched suites passed after them. tsc --noEmit and Biome are clean.

Upgrade steps

None: no schema, migration, env or deployment change.

JNO added 2 commits October 29, 2026 14:45
Rules: colour means an open view, a state or a person, never decoration;
action icons are neutral until their view is open; one colour per view;
counts at the right end of full-width rows and right after block headings.

- Block-heading counts render through one HeadingCount (muted, small, no
  parentheses): Task review and Learn sections, profile groups, stored
  remarks, both agenda sections, folder task groups, a person's recordings,
  billing groups.
- Full-width rows put their count at the right end: Needs review filter,
  agenda disclosure (wraps at narrow widths), arrival overflow, the
  transcript chapter bar (title first, position at the right).
- Neutral at rest: item header Share (magenta only for an existing person
  share) and Knowledge; folder menu agenda and share icons; item menu share;
  received-item filing; delete/revoke icons and destructive menu items, red
  only on hover/focus (contrast >= 4.5:1 in both themes).
- Item header Knowledge takes --knowledge and a pressed look while open.
- Inline editors keep their trigger visible but inactive while open
  (InlineEditorTrigger): task Note, meeting note, Edit summary, Add person,
  Add thing, nickname; focus returns to the trigger after save or cancel.
- Tasks orange only on links into Tasks; Task review labels, note borders and
  disclosures (agenda, show dropped) stay neutral; formatted mail and the
  access/archived-profile toggles use the neutral pressed look.
- Access settings ignore stale detail responses when switching people.
- Guide text for the inactive editor triggers; screenshots retaken where the
  UI changed; capture.sh follows the new Needs review label.
Switching to another person while a change for the first one was saving
dropped its error, success and session messages. The outcome of a write the
admin made is now always reported (a sign-in prompt applies to the whole
page); only the first person's detail is no longer reloaded or shown.
@claude

claude Bot commented Oct 29, 2026

Copy link
Copy Markdown

Review: feat(ui): apply the view colour and count rules across the UI

Large but mechanical PR (70 files, mostly styling/aria swaps to the new HeadingCount and InlineEditorTrigger primitives) plus a genuine race-condition fix in access settings and doc/screenshot updates. Reviewed by splitting across the shared primitives, the access-settings race fix, tasks/meetings, people/almanac/settings, and dashboard/mail/billing areas, each read in full against origin/dev and cross-checked against the specific design rules and "reviewer calls" the PR description states.

Verdict: COMMENT

Findings: Blocker 0 | Critical 0 | Major 1 | Minor 0 | Nit 0

Findings

  • MAJOR - src/components/ui/inline-editor-trigger.tsx:25-31 (inline comment posted): the aria-disabled hover override only neutralizes background-color, not text/icon color. For the ghost/outline/link branch, buttonVariants already bakes in hover:text-accent-foreground for ghost/outline (src/components/ui/button.tsx:17,20); that class targets a different CSS property than aria-disabled:hover:bg-transparent, so tailwind-merge doesn't drop either one. Concretely, hovering "Add thing" (things-list.tsx, variant="outline"), "Add a nickname" (known-facts.tsx, variant="ghost") or "Edit summary" (summary-widget.tsx, variant="ghost") while their editor is open still visibly changes the trigger's text/icon color, contradicting the PR's own rule that the trigger must "stay visible but inactive" while open. task-row.tsx/agenda-body.tsx dodge this only incidentally, by passing their own hover:text-foreground override. src/tests/ui/ui-rules-v2.test.tsx only asserts the bg-transparent override, not a text-color one, so this shipped with a green suite. Suggested fix: add aria-disabled:hover:text-foreground to that branch.

Verified clean (no issues)

  • HeadingCount and the dropdown-menu.tsx destructive variant: correct, and destructive-menu-contrast.test.ts's OKLCH/WCAG contrast math is genuinely computed against the real tokens, not a tautology.
  • The access-settings race fix (access-section.tsx): traced by hand against A->B, A->B->A, and double-click-reload interleavings - the {subject, generation} ref and isCurrent() staleness checks are correct; a write's outcome is always reported while only the stale detail-panel mutation is suppressed, matching the PR description exactly. access-selection.test.tsx asserts real rendered content for these races, not just "no crash." (Non-blocking note: stale fetches are discarded by ref comparison rather than AbortController-cancelled, unlike person-actions.tsx elsewhere in the codebase - fine here, just an inconsistency if this pattern gets reused.)
  • Tasks/meetings area (agenda-body.tsx, summary-widget.tsx, task-review-dialog.tsx, task-row.tsx, task-detail.tsx, recording-tasks.tsx, folder-tasks-panel.tsx, summary-remarks-dialog.tsx): all the PR's explicit "reviewer calls" (neutral task-review labels/title icon, aria-expanded-only disclosures, neutral note borders, Tasks-orange restricted to links into Tasks, agenda row layout) are implemented as described; one-body-per-widget confirmed via the describe.each(["inline","dialog"]) test.
  • People/almanac/settings area (things-list.tsx, known-facts.tsx, people-list.tsx, person-detail.tsx, profiles-section.tsx, plaud-account-section.tsx, api-keys-section.tsx, template-list.tsx, webhooks-section.tsx, folder-sharing.tsx, person-sharing.tsx, settings-nav-sidebar.tsx): Organization-colour, destructive-neutral-at-rest, and InlineEditorTrigger focus-return are all correct; no new hardcoded strings, no real names in fixtures.
  • Dashboard/mail/billing area (arrival-notice.tsx, folder-tree.tsx, recording-list-toolbar.tsx, recording-row.tsx, transcript-topics-menu.tsx, item-header.tsx, learn-widget.tsx, mail-detail-pane.tsx, help-drawer.tsx, hosted billing page): the transcript-chapter-bar title/position reorder and the Knowledge-icon pressed-state fix (previously always purple for Organization items regardless of open state) are both correct and covered by updated tests.
  • Docs twins (almanac.md/.cs-CZ.md, summaries-and-tasks.md/.cs-CZ.md) are in sync, capture.sh follows the new "Needs review 1" label; no production/real-name leaks spotted.

Missing tests

None beyond the text-color gap above (ui-rules-v2.test.tsx would need a hover:text-* assertion for ghost/outline/link triggers once fixed).

The inactive trigger only reset its hover background, so ghost and outline
triggers (Edit summary, Add thing, Add a nickname) still changed text colour
on hover. While its editor is open the trigger now has pointer-events: none:
no hover state of any variant, keyboard focus unchanged, clicks were already
ignored.
@claude

claude Bot commented Oct 29, 2026

Copy link
Copy Markdown

Review: feat(ui): apply the view colour and count rules across the UI

Re-review after the last push (385c0387). No new commits landed beyond the two already covered by the prior full review (3401cec6, 2704c7a3) and the fix commit itself; HEAD merges cleanly with no diff against 385c0387.

Verdict: APPROVE

Findings: Blocker 0 | Critical 0 | Major 0 | Minor 0 | Nit 0

Findings

None. The sole MAJOR from the prior round (src/components/ui/inline-editor-trigger.tsx: the aria-disabled hover override only neutralized background-color, leaving ghost/outline text/icon colour changing on hover while an editor was open) is fixed in 385c0387: the component now applies aria-disabled:pointer-events-none instead of per-variant background overrides, which blocks :hover entirely regardless of variant, dropping the variant-branching logic along with it. Keyboard focus and the existing onClick guard are untouched, matching the stated intent. ui-rules-v2.test.tsx was updated to assert aria-disabled:pointer-events-none in place of the old hover:bg-* checks, including the no-variant-branch case. Confirmed by re-reading both files in full; nothing else changed in this push.

Missing tests

None beyond what the prior round already covered.

@novoj
novoj merged commit 203db93 into dev Oct 29, 2026
8 checks passed
@novoj
novoj deleted the feat/ui-rules-v2 branch October 29, 2026 15:14
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