Skip to content

fix(sync): remediation pass on Sync tab redesign stack#967

Merged
kunickiaj merged 1 commit into
mainfrom
04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_
Apr 23, 2026
Merged

fix(sync): remediation pass on Sync tab redesign stack#967
kunickiaj merged 1 commit into
mainfrom
04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_

Conversation

@kunickiaj
Copy link
Copy Markdown
Owner

@kunickiaj kunickiaj commented Apr 23, 2026

Description

Remediation PR on top of #963#966. Addresses the findings from four
brutal reviews across the stack. Batched here so reviewers see one
coherent "Ramsay fix" diff instead of amending four in-flight PRs.

Bugs fixed

Doc drift fixed (#966)

  • Removed phantom `--pip-size-sm` / `--pip-size-md` tokens
    (PresencePip sizes are just `6 | 8` literals).
  • Removed `--accent-warm-strong` implication that attention pips
    use a distinct hue — they don't, the CSS falls back to
    `--accent-warm`. Pulse alone distinguishes attention from offline.
  • `DeviceRow` props block relabeled as a proposed interface; the
    real component is still `SyncPeerCard`.
  • Deleted `action-shelf-anatomy.md` (no consumer primitive).
  • Updated "don't nest drawers" anti-pattern to acknowledge that
    inline sub-disclosures inside a drawer are fine
    (`PeerScopeCollapsible` inside the device drawer is the example).
  • Focus-management claim clarified as emergent, not designed.

Code smell sweep (#964)

  • Deleted underscore-prefixed dead vars (`_effectiveInclude`,
    `_effectiveExclude`, `_inheritsGlobal`, `_summaryText`).
  • Moved the mid-file `renderIntoSyncMount` import to the top.

Type of Change

  • Bug fix
  • Code cleanup
  • Documentation correction

Testing

  • `pnpm run tsc && pnpm run lint && pnpm run test` (1475 passing,
    386 files linted — 2 fewer than before thanks to the primitive
    deletions).
  • Manual next step: bookmark `#sync/diagnostics`, reload, verify the
    sub-view loads; tab through the device row, verify only the chevron
    receives focus.

Stacks on #966. Closes out the Ramsay review feedback across the
redesign stack.

@kunickiaj kunickiaj changed the title fix(sync): address Ramsay review of Sync tab redesign stack (PR 5/4) fix(sync): address Ramsay review of the Sync tab redesign stack Apr 23, 2026
@kunickiaj kunickiaj marked this pull request as ready for review April 23, 2026 22:41
@kunickiaj kunickiaj changed the title fix(sync): address Ramsay review of the Sync tab redesign stack fix(sync): remediation pass on Sync tab redesign stack Apr 23, 2026
@kunickiaj kunickiaj force-pushed the 04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_ branch from 57ee882 to 501ae9d Compare April 23, 2026 23:33
Copy link
Copy Markdown
Owner Author

kunickiaj commented Apr 23, 2026

Merge activity

  • Apr 23, 11:35 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 23, 11:48 PM UTC: Graphite rebased this pull request as part of a merge.
  • Apr 23, 11:49 PM UTC: @kunickiaj merged this pull request with Graphite.

@kunickiaj kunickiaj force-pushed the 04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_ branch from 501ae9d to edd79c5 Compare April 23, 2026 23:44
@kunickiaj kunickiaj force-pushed the 04-23-docs_design_sync_tab_redesign_handoff_anatomy_pages_pr_4_4_ branch from 7aa2173 to 0eee1da Compare April 23, 2026 23:44
@kunickiaj kunickiaj changed the base branch from 04-23-docs_design_sync_tab_redesign_handoff_anatomy_pages_pr_4_4_ to graphite-base/967 April 23, 2026 23:46
@kunickiaj kunickiaj changed the base branch from graphite-base/967 to main April 23, 2026 23:46
@kunickiaj kunickiaj force-pushed the 04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_ branch from edd79c5 to 8fa568c Compare April 23, 2026 23:47
@kunickiaj kunickiaj merged commit e4e9ba5 into main Apr 23, 2026
8 checks passed
@kunickiaj kunickiaj deleted the 04-23-fix_sync_address_ramsay_review_of_sync_tab_redesign_stack_pr_5_4_ branch April 23, 2026 23:49
kunickiaj added a commit that referenced this pull request Apr 24, 2026
## Description

Dogfood of the merged Sync tab redesign (PRs #963-#967) surfaced five real
issues. Fixing them together since they're all the same "Sync tab polish"
story.

- **Device list was a 2-column grid above 900px**, so expansion rendered
  inside a narrow sidebar column while the other row sat empty. Switched
  to single-column full-width rows; outer border/radius/background now
  live on `.peer-card`, the row head is transparent content, and the
  drawer has a `border-top` separator inside the same card.
- **Row head was a grid that broke when the direction glyph was absent**,
  causing the chevron to wrap to a second line. Switched to flex;
  `.device-row-toggle { margin-left: auto }` keeps the toggle right-edge
  regardless of which middle children render.
- **Row expand toggle was a hand-rolled `<button>` with a unicode ▸/▾**
  that was nearly invisible against `--surface-1`. Replaced with Radix
  Collapsible (`@radix-ui/react-collapsible`, already installed) + a
  properly sized inline Lucide chevron SVG that rotates on `data-state="open"`.
  Radix gives us correct `aria-expanded`/`aria-controls` wiring and
  keyboard handling for free. Module-level `expandedPeerId` still
  enforces single-open-at-a-time; `handleOpenChange` reads the live
  module state (not a stale closure) so one row taking over doesn't
  clobber the new pointer.
- **`View diagnostics →` and `← Back to sync` had different styles**.
  Collapsed into one `.sync-subview-link` class used for both
  directions — utility-nav treatment, quieter than any nearby button.
- **`work is offline` surfaced as a Needs Attention item.** Computers
  power-cycle; the row already carries the Offline badge + pip. Dropped
  the `peerStatus === "offline"` branch from `sync-view-model.ts`'s
  attention-item derivation. Auth/trust/repair failures still fire.
- **Pairing payload was double-hidden** ("Show pairing command" →
  revealed a card that just said "Pairing payload hidden" unless
  diagnostics were on). The payload is a local command shared with your
  own other device, not a secret. The outer disclosure is enough
  gating; removed the redaction branch in `diagnostics/helpers.ts`.
- **Sync tab cards had no gap between them** while Feed did. The
  redesign's `#syncMainView` / `#syncDiagnosticsView` wrappers broke
  the `.tab-panel` flex-gap inheritance; restored the same gap inside
  each sub-view wrapper.

## Type of Change

- [x] Bug fix
- [x] UX polish

## Testing

- \`pnpm run tsc && pnpm run lint && pnpm run test\` (1475 passing)
- Manual: Sync tab — device row toggle is visible mint-on-surface
  chevron that rotates 90° on open; expand/collapse one row cleanly
  stacks in a single column; View diagnostics / Back to sync read as
  the same utility link; offline peer no longer shows up in Needs
  attention; "Show pairing command" reveals the command directly.
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