Feature/minor upgrades - #183
Conversation
The aurora background draws a light trail behind the pointer, and every number that shaped it was a magic constant inside the effect closure: the trail tapered from a fixed 60px line width, at a fixed peak alpha of 0.36. Nobody could turn that down short of disabling the whole effect, which was all-or-nothing for what is fundamentally a matter of taste and screen size. This adds a single persisted setting, glowIntensity, running 10-100: - ThemeContext declares it; ThemeProvider owns the state, clamps reads and writes into [10,100] (a hand-edited localStorage value must not be able to break the effect), and persists under the existing sprintstart:* key convention. Default is 50, so turning the effect on lands at a moderate glow rather than full blast - the effect is opt-in, and opting in should not mean maximum. - AuroraBackground scales both knobs from it: radius as 60 * t * life and alpha as life * 0.36 * (0.25 + 0.75 * t). The alpha floor keeps the lowest setting visible, because the radius shrink already does most of the dimming work - without the floor, 10% meant an invisible trail rather than a small one. - The value reaches the animation loop through a ref synced in a useEffect, not an effect dependency. The rAF loop then reads the latest value every frame, so dragging the slider changes the live trail immediately while the pointer listener stays mounted - wiring it as a dependency instead would tear the listener down and clear the trail mid-gesture on every step of the drag.
The intensity setting added in the previous commit had no surface.
It lives in Settings > Appearance, directly under the Aurora
Background toggle, and only appears while that toggle is on - a
slider for a disabled effect would be noise, and hiding it keeps the
opt-in pattern honest: enable first, then tune.
The control itself is deliberately plain. This is the app's first
range input, so it uses the native control tinted with
accent-app-brand rather than hand-built track CSS; if a second slider
ever arrives, that is the point to extract shared styling, not
before. Steps run in increments of ten - ten discrete stops are easy
to hit precisely, while a 91-value drag would make fine adjustment
fiddly without making the result perceptibly different. The current
value renders beside the label ("Glow intensity · 50") with tabular
numbers so the readout does not wiggle while dragging.
Changes preview live: the effect reads the context value every frame,
so there is no apply or save step to forget.
Tests cover the three behaviors that matter: the slider is absent
while aurora is off, it appears at the default 50 once the toggle is
flipped, and changing it persists to localStorage and updates the
readout.
The escalation inbox predated several of the app's shared UI primitives and kept its own hand-rolled versions after those primitives landed. This migrates it onto the real ones, pattern by pattern. Tabs. The page built its Open / Durable answers switcher from a local TabButton: an underline bar with a border-bottom active state. Every other page had moved to SegmentedTabs - the sliding brand pill with hover magnify - leaving this the one page whose section switcher looked and behaved differently. It now uses SegmentedTabs (with its own layoutId, since Framer matches those globally), and the panel content sits inside SlidingTabPanel like its siblings, so switching views slides directionally instead of flicking. Counts still stay hidden while their list loads, and the aria-label carries over. Empty states. The page defined a private EmptyState with a solid border and p-16 padding - one more of the hand-written variants the UI consistency work replaced. It now uses ui/EmptyState, so an empty inbox looks like every other empty state in the app: dashed border, shared spacing, body copy as children. The distinction between "no escalations" and "still loading" continues to come from the words. Loading. The View helper rendered a raw Loader2 with no accessible announcement - a screen reader user got silence while lists loaded. It now uses ui/Spinner with specific labels, which announces the wait through role=status once, app-wide. Buttons. RequestCard, AnswerForm and CanonicalAnswerCard spelled their action buttons by hand six times over: bg-app-brand plus hand-picked padding, hover, and disabled classes. Beyond the drift itself, those buttons had no focus-visible ring (invisible keyboard focus), no aria-busy while saving, and inconsistent disabled opacity. They are all ui/Button now - primary for the main action, secondary for cancel, ghost for dismiss - with loading states driven by the loading prop instead of string swaps like "Dismissing...". That deletes roughly forty lines of bespoke className and makes every action reachable, announced, and visually consistent.
The page had no test coverage at all - the only knowledge-request test exercised FlagToPmButton. After the migration onto the shared patterns, these tests pin the behaviors that migration must not regress: - Open escalations render under the default tab with their count, and the switcher exposes them the way SegmentedTabs actually does: a group of aria-pressed toggle buttons, not an ARIA tablist (a deliberate choice documented in the component itself). - An empty inbox and a still-loading one differ by their words, not their shape - the rule from the UI consistency work that keeps the page from visibly rebuilding itself when data arrives. - The Spinner announces the wait through role=status while lists load, and stops once they have arrived. - Answer and Dismiss remain real buttons, findable by accessible name, so future styling can never again trade away semantics. - A reader without write permission sees durable answers but no Edit affordance - the read-only hint is the only trace of editing. The service layer is mocked wholesale; project selection comes through the shared createProjectContextValue test helper, with one project present so the page renders past its no-project guard.
- Scale trail peak line width from 6px at 10% to 90px at 100% intensity - Increase base alpha to 0.28 at 10% (2x saturation) and 0.70 at 100% (intense radiance) - Expand canvas OVERSCAN buffer to 45px to prevent stroke edge clipping
- Add .app-scrollbar utility with sleek rounded thumb matching theme border tokens - Pin sidebar header and user profile/logout footer with shrink-0 - Enable min-h-0 and overflow-y-auto on navigation container for long route lists - Listen to captured scroll events in SidebarNavLink for accurate dock magnification
…ox, and settings - AuroraBackground: move OVERSCAN to module scope and remove from useEffect dependency array - RequestCard: ensure setDismissing(false) is resolved on success in addition to error - CanonicalAnswerCard: migrate read-only edit trigger button to shared Button primitive - AnswerForm: reset submit status to 'idle' upon successful submission - AppearanceSection: change glow intensity slider step to 1 for continuous adjustment - KnowledgeRequestInboxPage: add comment guard for TAB_ORDER and Tab union synchronization - KnowledgeRequestInboxPage.test: scope mock profile mutation within nested describe and afterEach
|
Read all 7 commits. The design-system migration in the escalation inbox is a clean win — and dropping Blocking-ish: the sidebar's new scroll listener re-measures every nav row on every scroll in the app
window.addEventListener("scroll", measure, true);Capture phase on The comment directly above that effect is the argument against doing this:
Also note the effect has no dependency array (deliberate, per the same comment), so both listeners are now torn down and re-registered on every render of every row. Options, cheapest first:
Question: the default glow got brighter than what everyone has todayBefore this PR the trail was fixed at So a user who never opens Settings gets a noticeably stronger glow than before — which cuts against the reasoning in Nits
Not approving/merging for now. Reviewed with Claude Code. |
- SidebarNavLink listened for scroll on `window` in the capture phase, so every scroll anywhere in the app — main content, drawers, tables — ran `measure()` once per nav row, each forcing a layout via `getBoundingClientRect`. That is the exact per-entry cost `centerYRef` was introduced to avoid. It now listens on the sidebar's own scroll container (marked `data-sidebar-scroll` on the nav) plus bubble-phase window scroll for the document scroller, and coalesces bursts into one measurement per frame with requestAnimationFrame. - KnowledgeRequestInboxPage: `TAB_OPTIONS` was a per-render local in module-constant casing, rebuilt on every render and handed to SegmentedTabs. Now a memoized `tabOptions`. - The shared `View` helper announced "Loading escalations" on both tabs; the label is a prop, so the durable-answers tab says what it is loading. - GLOW_INTENSITY_* constants move from ThemeProvider.tsx to ThemeContext.ts, so the settings slider and the useTheme fallback can read the bounds without importing the provider module. useTheme's fallback now uses the shared default instead of a hardcoded 50, and keeps one spelling for its no-op setters. - Glow slider: add aria-valuetext so a screen reader announces "50%" rather than a bare "50" — the visible readout already carries the unit. Tests: 1870 passed (256 files). tsc -b, eslint, vite build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed and pushed a follow-up commit to this branch. One performance fix, the rest is polish — plus one thing I deliberately did not change, see the bottom. The one that matters: the sidebar re-measure window.addEventListener("scroll", measure, true);Capture phase on That's the exact cost the ref right above it was introduced to avoid — the existing comment says so:
Now it listens on the sidebar's own scroll container (the nav is marked Polish, same commit
Not changed — your call The aurora rescale in
So a user who never opens Settings gets a thinner but ~30% more opaque trail than today, and the top of the range is roughly double the old alpha. That reads like a deliberate "make it pop" choice, but it's worth being explicit about: commit 1's rationale was that the default should be "a moderate glow rather than full blast", and the rescale in commit 5 quietly moved what "moderate" means. Changing the visual default is your decision, not mine, so I left it — just flagging that it isn't a no-op for existing users. Verified on the branch: The escalation-inbox migration onto |
Backmerge before review: the branch forked at d815ae2 and dev has moved 17 commits since. Merged clean, no conflicts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DavidLeuter
left a comment
DavidLeuter
left a comment
There was a problem hiding this comment.
Approving. Re-read the branch as it stands (f37d1d9) and tested it on top of today's dev — it forked at d815ae2 and dev has moved 17 commits since, so I wanted the merge result green rather than just the branch.
Backmerge verified locally
Merged origin/dev into the branch in a scratch worktree. Clean, no conflicts. Two files are touched by both sides and both turned out to be genuinely disjoint:
SideBar.tsx—devaddedisAssistantSectionActiveso the Chat entry lights up on/buddy; this branch changed the layout classes on the wrapper, the<nav>and the footer card. Different hunks, no shared logic.index.css—devappended the widget-wiggle keyframes at the end of the file, this branch inserted.app-scrollbarmid-file. No name collision, and--border/--border-strongare defined in both the light and dark blocks, so the thumb has a real colour in either theme.
On the merge result: tsc -b, eslint ., vite build clean, 1900/1900 tests pass across 260 files. No flakes on this one.
The scroll fix holds up against what dev added
Worth saying explicitly, because dev shipped two new scrolling surfaces after this branch was written — ConversationRail and the chat/buddy AssistantShell. The re-measure is scoped to closest("[data-sidebar-scroll]") plus bubble-phase window scroll, so neither of them re-triggers it; had this still been window + capture phase, the merge would have handed every one of those rails a dozen forced layouts per scroll event. The scoping was worth doing before those landed rather than after.
Still open, and still your call: the default glow got brighter
Unchanged since the last round, so restating it once rather than blocking on it. sliderProgress() maps 10–100 onto 6→90px width and 0.28→0.70 alpha, which puts the default of 50 at ~43px / α≈0.47:
| line width | alpha | |
|---|---|---|
| before this PR (fixed) | 60px | 0.36 |
| now, at the default 50 | ~43px | ~0.47 |
| now, at 100 | 90px | 0.70 |
So somebody who never opens Settings gets a thinner but ~30% more opaque trail than they have today — the setting isn't purely additive. That reads like a deliberate "make it pop", and it cuts against commit 1's own rationale that the default should land on "a moderate glow rather than full blast". Mapping 50 back onto 60px / 0.36 would make it additive and change nobody's current experience. Either answer is fine; it's a visual-default decision, not a defect, which is why it isn't holding up the approval.
The rest
The escalation-inbox migration onto SegmentedTabs / SlidingTabPanel / EmptyState / Spinner / Button is the strongest part of the PR — bespoke markup deleted rather than moved, and the new test suite means it stays that way.
Merge order per the description is first of the four, before #184 and #185. Given how far dev has moved, backmerge before you merge.
Reviewed with Claude Code.
2 participants
Note
Merge & Review Sequence (Step 1 of 4)
dev(Independent foundation)dev. Downstream PRs (Easter Eggs Consolidation & Overhaul #184 and Feature/269 confluence connector #185) depend on these changes.📌 Summary
This PR delivers focused frontend enhancements improving visual polish, design consistency, accessibility, and navigation usability across three core areas:
localStoragepersistence.SegmentedTabs,SlidingTabPanel,EmptyState,Spinner,Button) backed by full unit test coverage.🛠️ Key Changes & Architectural Improvements
1. Aurora Background & Glow Customization
ThemeContext.ts,ThemeProvider.tsx,useTheme.ts):glowIntensity(range10–100, default50) andsetGlowIntensity().localStorageundersprintstart:glow-intensity.AppearanceSection.tsx):tabular-nums) to prevent label shift while dragging.AuroraBackground.tsx):6pxline width with increased alpha (0.28, ~2x color saturation).90pxline width with high peak alpha (0.70, vibrant radiant aura).OVERSCANbuffer from30pxto45pxto avoid edge clipping.glowIntensityRefto avoid rAF loop teardowns during drag interactions.2. Escalation Inbox UI Modernization (
src/features/knowledge-request/)KnowledgeRequestInboxPage.tsx):SegmentedTabsandSlidingTabPanelfor smooth directional transitions.EmptyState.Loader2with accessibleSpinner(role="status").<button>markup acrossRequestCard.tsx,AnswerForm.tsx, andCanonicalAnswerCard.tsxwith standardButtonvariants (primary,secondary,ghost).3. Scrollable Main Navigation Sidebar
SideBar.tsx):shrink-0.flex-1 min-h-0 overflow-y-auto app-scrollbarto allow smooth scrolling for long route lists.src/styles/index.css):.app-scrollbarutility with a subtle5pxrounded pill thumb.SidebarNavLink.tsx):🗂️ Modified Files Summary
src/components/layout/AuroraBackground.tsx6px–90px,0.28–0.70alpha), 45px overscansrc/context/ThemeContext.tsglowIntensitysrc/context/ThemeProvider.tsx10–100), default value (50)src/context/useTheme.tssrc/features/settings/components/AppearanceSection.tsxsrc/features/knowledge-request/components/KnowledgeRequestInboxPage.tsxSegmentedTabs,SlidingTabPanel,EmptyState,Spinnersrc/features/knowledge-request/components/RequestCard.tsxButtonprimitivesrc/features/knowledge-request/components/AnswerForm.tsxButtonwith loading state & iconssrc/features/knowledge-request/components/CanonicalAnswerCard.tsxsrc/styles/index.css.app-scrollbarutilitysrc/components/layout/SideBar.tsx<nav>, pinned header/footer elementssrc/components/layout/SidebarNavLink.tsxtests/unit/features/settings/AppearanceSection.test.tsxtests/unit/features/knowledge-request/components/KnowledgeRequestInboxPage.test.tsx🧪 Verification & Quality Assurance
Automated Checks
prettier --check .tsc -b && vite buildeslint .vitest runManual Verification
localStorage.📋 Definition of Done (DoD)
role="status"on spinners and focus rings on all buttons.💡 Reviewer Guidance: Please review with Claude Code and commit/push any fixes directly to this branch.