feat(web): simplify Space navigation and account settings#26
Merged
Conversation
johannesjo
force-pushed
the
task/beautify-02-entry-routes
branch
2 times, most recently
from
July 21, 2026 10:07
22ced02 to
22eddbf
Compare
Add a direct Spaces overview link, put account and Space-management controls behind clear disclosures, and defer verified account Space loading until the account section opens. Keep affected browser flows aligned with the new navigation.
The People panel grew a third copy of the "section header + Show/Hide button + hidden body" markup, so lift it into a shared primitive that owns the aria-expanded/aria-controls wiring and derives its test ids. Documented in the styleguide next to Collapsible, with the split spelled out: Collapsible folds a content card on its title row and persists per device; DisclosureSection is a per-visit show/hide for settings. Also: - drop the panel's duplicate "People" heading — the dialog label, an h2 and an h3 all said it; the online count moves up to the h2 - remove tabindex="-1" from the Space title; nothing focuses it - cover the new header back-link with an e2e test - rename Header's .overviewLink to .backLink so it stops colliding with MemberList's unrelated class of the same name - explain why .linkButton takes the hover accent at rest, and why accountOpen reads its prop once - stop Header.test leaking a window.matchMedia stub Verified: 174 web unit tests, tsc, eslint, prettier, production build.
In flow the back-link added 18.06px to a header that is 86.14px tall on a Pixel 5 — 21% taller — and at <=600px the header is position: static with .shell padding-top: 0, so every pixel lands on page content. That pushed the checklist card past the 851px fold, which is why the mobile cross-card drag specs fail: dragRowAbove aims mouse.move() at a raw boundingBox() with no scroll-into-view, so an off-viewport target never receives the pointer and the insertion poll times out. Taking the link out of flow parks it in the top padding .inner already reserves. Measured at Pixel 5: header back to 86.140625px, identical to before the link existed, with the link still visible at the same 14.06px hit height (now 70.9px wide instead of 62.6px). DOM order is unchanged, so tab and screen-reader order are unaffected. Desktop is untouched.
johannesjo
force-pushed
the
task/beautify-03-member-navigation
branch
from
July 21, 2026 11:16
2c8135a to
46b3a5f
Compare
johannesjo
marked this pull request as ready for review
July 21, 2026 11:28
johannesjo
added a commit
that referenced
this pull request
Jul 21, 2026
PR #26 removed this tabindex on the grounds that nothing focused the Space title. That was true at the time; this PR makes it false -- the project load's retry effect focuses `[data-testid="project-name"]` once a reload succeeds. focus() on a plain h1 is a silent no-op, and Project.test mocks Header with its own focusable h1, so without this the retry flow would break with the whole suite still green. The Header contract test added in this PR catches exactly that, and comment names the reason so the attribute does not get tidied away again.
johannesjo
added a commit
that referenced
this pull request
Jul 21, 2026
* feat(web): clarify the Project shell and card hierarchy Add meaningful loading, error, retry, and terms states; preserve modal focus ownership; expose card headings and reading order; and reduce unused Scratchpad height. Exercise the real reactive store with typed API fixtures. * fix(web): scope project load error alert * fix(web): harden Project shell layout and card semantics Replace the float-based content layout with a grid: an explicit flexible second row keeps Recent activity pinned under the task list when the Scratchpad column outgrows it, and `align-content: start` stops grid from padding the stacked cards apart when the shell is taller than its content. Both behaviours are browser-measured; the columns now resolve to the exact 1.35fr / 1fr split rather than the floats' percentage approximation. Restore a usable Scratchpad editor on engines without `field-sizing` (Baseline only since June 2026) -- with `resize: none` and a 64px floor those users had a three-line scroller and no way out. Drop the boolean `handleDeleteItem` returned: both consumers type the callback as `void` and discard it, so nothing could observe it. The two tests that asserted it now assert the API call and the toasts instead. Give ListCard the section heading and `aria-controls` the other cards gained, take the terms gate back off `<main>` (Dialog renders through a Portal, so that landmark was always empty), and drop a `tabindex` and an `aria-live` that were dead. * test(web): lock the Space name as a focusable heading Project's retry flow focuses `[data-testid="project-name"]` after a successful reload, but Project.test.tsx mocks Header with a hand-written h1 -- so dropping the real tabindex would break the flow with every test still green. Assert the contract where Header itself is rendered. * test(e2e): assert the hero list heading and collapse wiring ListCard gained the section heading and aria-controls the other cards already had, but it has no unit test file -- so cover it where the real card is rendered, alongside the existing collapse assertions. * style(web): reset heading defaults on the list card title * fix(web): keep the Space title focusable for the retry flow PR #26 removed this tabindex on the grounds that nothing focused the Space title. That was true at the time; this PR makes it false -- the project load's retry effect focuses `[data-testid="project-name"]` once a reload succeeds. focus() on a plain h1 is a silent no-op, and Project.test mocks Header with its own focusable h1, so without this the retry flow would break with the whole suite still green. The Header contract test added in this PR catches exactly that, and comment names the reason so the attribute does not get tidied away again. * fix(web): settle retry focus and align the card heading resets Multi-agent review turned up three defects and one duplication. The retry flow released focus too early: setProjectData clears `loading` as soon as the project lands, but the load is still fetching activity, so a rejection there swapped in the error screen after focus had already committed to the Space title -- leaving it on an unmounted heading. Gate the final move on a new `loadSettled` signal that only the current load may set, and cover it with a test that fails without the gate. Batch handleRetry's three writes. Unbatched, the focus effect first ran against a torn state and only survived because queueMicrotask deferred it until the stale node had gone; with a coherent state the microtask hop is unnecessary, so the flag lifecycle no longer depends on a selector miss. The Scratchpad's `field-sizing` fallback raised only the editor, not the display button it swaps with, so engines without `field-sizing` jumped ~156px on click. Both now read one `--pad-min-h`. Drop `rows`, which the spec ignores under `field-sizing` and which the fallback floor outranks otherwise, and stop the test asserting it. PanelCard's h2 wrapper is `display: flex` again: the rename input is `flex: 1`, which a block parent silently ignored, dropping it from 403px to its intrinsic 295px and overflowing narrow cards. Both card headings also get ListCard's `font: inherit`, without which global.css's `button { font: inherit }` pulls the h2's bold into a collapsed card's "N" count. Finally, use the shared useDocumentTitle helper instead of a bespoke effect -- every other route already does. * refactor(web): drive retry focus from the load, not from state The previous fix inferred "the load has finished" from a third signal (`loadSettled`) and a 25-line effect that re-derived which screen was up from `loading`/`error`/`project`/`termsRequired` -- a second source of truth for the routing the <Show> tree already owns. The load itself knows when it settles, so call focus from there: one `retryFocusPending` boolean (no longer a signal -- nothing renders from it), a two-line focus helper, and a `settleRetryFocus()` invoked from the load's existing guarded `finally`. Behaviour is unchanged and the same regression test still fails when focus settles before the activity fetch. Dropping the effect also removes the need for `batch()` and the explicit `setLoading(true)`: with no effect observing the intermediate state, the load effect's own `resetState()` renders the loading screen before `focus()` looks for it. Net -19 lines, one fewer signal, one fewer effect. * fix(web): untrack the retry-focus hand-off inside the load effect The no-identity branch returns before any `await`, so the load IIFE's `finally` runs synchronously while the effect body is still on the stack with a tracking Listener. `settleRetryFocus()` reads `state.error` and `termsRequired()`, which therefore subscribed the *load* effect to them: with a retry pending, the next `setError` re-ran the whole teardown -- abort, disconnectSSE, resetState and a second navigate. Wrap the call in `untrack`. Covered by a test that fails without it. Also correct both heading comments, which named the wrong mechanism. Measured on the built app: the reset matters because a *block* h2 gives its line box a 1.5em/bold strut -- ScratchpadCard's header is 27px with the reset and 36px without. PanelCard is immune to that anyway now that its heading is a flex container, so there the reset only keeps the UA font off the toggle. Neither card passes `count` today, so the bold "N" the old comments cited was latent, not the actual reason. * refactor(web): hand the collapse body id out with the collapse state Three cards were pairing `createCollapsed(id)` with a separate `createUniqueId()` and then threading the result into both `CollapseToggle`'s `controls` and `CollapseBody`'s `id` -- the kind of two-step a card can silently half-wire, which is why the e2e assertion guards it. `createCollapsed` now returns `bodyId` alongside `collapsed`/`toggle`, so the toggle and the body it folds can only come from the same source. DisclosureSection keeps its own `createUniqueId`: it is the per-visit show/hide primitive and does not use `createCollapsed` at all. Also drop `.emptyState` from Project.module.css (no TSX has referenced it for some time) and record that the shared scratchpad floor depends on the global border-box reset for the display/editor heights to match exactly.
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
1 participant
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.
Summary
Why
Account controls competed with the primary member view, and hidden account data was loaded before it was needed.
Impact
The Space header and account area are easier to scan, while preserving verification and recovery behavior.
Validation
Stack 3 of 6. Base:
task/beautify-02-entry-routes. Review and merge this stack from the bottom upward.