fix: Alerts editor layout, collapsible linked accounts, and test alert #107

Merged
Gitchegumi merged 1 commit from fix/alerts-editor-layout-fixes into main 2026-07-06 14:52:08 -05:00
Gitchegumi commented 2026-07-06 12:54:35 -05:00 (Migrated from github.com)

Addresses four issues on the Alerts editor page.

Changes

1. Canvas jump + non-scrollable Alerts tab
The left panel is a grid item without min-height: 0, so tall tab content (the Alerts list) grew the shell's middle 1fr row past the viewport, pushing the stage down and clipping everything below the fold — which also prevented the list itself from scrolling. Capping the panel height lets the existing overflow: auto on the tab panel take over. The Alerts list now scrolls and the canvas stays anchored to the top when switching tabs.

2. Collapsible linked accounts
LinkedAccountBanner gains a collapsible prop (enabled on the Alerts page). It renders collapsed by default as a single header row with a connected-count summary (e.g. "Linked accounts · 2 connected") and a chevron toggle, and auto-expands after an OAuth connect/error so the result is still visible. Default behavior elsewhere is unchanged.

3. Test alert button did nothing visible
It fired manual.test (which bound canvases ignore, so nothing rendered in OBS) and the editor "preview" only swapped placeholder text values. The top-bar button now defaults to the first event key actually bound to the canvas (falling back to manual.test for unbound canvases), plays the canvas alert sound at its configured volume, and replays each element's entrance animation in the editor preview using the same keyframes as the live overlay. The per-alert Test buttons share this path.

4. Removed "Live in OBS" pill
It was styling on the profile's isActive flag with no real signal. Removed from the top bar along with its unused CSS.

Testing

  • pnpm --filter @multi-stream-alerts/web typecheck — passes
  • pnpm --filter @multi-stream-alerts/web test — 157/157 pass

🤖 Generated with Claude Code

Addresses four issues on the Alerts editor page. ## Changes **1. Canvas jump + non-scrollable Alerts tab** The left panel is a grid item without `min-height: 0`, so tall tab content (the Alerts list) grew the shell's middle `1fr` row past the viewport, pushing the stage down and clipping everything below the fold — which also prevented the list itself from scrolling. Capping the panel height lets the existing `overflow: auto` on the tab panel take over. The Alerts list now scrolls and the canvas stays anchored to the top when switching tabs. **2. Collapsible linked accounts** `LinkedAccountBanner` gains a `collapsible` prop (enabled on the Alerts page). It renders collapsed by default as a single header row with a connected-count summary (e.g. "Linked accounts · 2 connected") and a chevron toggle, and auto-expands after an OAuth connect/error so the result is still visible. Default behavior elsewhere is unchanged. **3. Test alert button did nothing visible** It fired `manual.test` (which bound canvases ignore, so nothing rendered in OBS) and the editor "preview" only swapped placeholder text values. The top-bar button now defaults to the first event key actually bound to the canvas (falling back to `manual.test` for unbound canvases), plays the canvas alert sound at its configured volume, and replays each element's entrance animation in the editor preview using the same keyframes as the live overlay. The per-alert Test buttons share this path. **4. Removed "Live in OBS" pill** It was styling on the profile's `isActive` flag with no real signal. Removed from the top bar along with its unused CSS. ## Testing - `pnpm --filter @multi-stream-alerts/web typecheck` — passes - `pnpm --filter @multi-stream-alerts/web test` — 157/157 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code)
PorteGumi (Migrated from github.com) approved these changes 2026-07-06 14:49:47 -05:00
PorteGumi (Migrated from github.com) left a comment

Dock, reviewed PR #107 — approving. Clean PR that addresses four real UX bugs on the Alerts editor page. CI is all green (typecheck, tests 157/157, lint, build, security scans), I re-ran typecheck and tests locally against the PR branch and both pass.

What the PR does

  1. Canvas jump + non-scrollable Alerts tab — Adds min-height: 0 to the left panel's grid shell. Correct fix for the CSS grid 1fr row overflow issue — without it, tall tab content expands the row past the viewport, pushing the stage down and clipping the list. The existing overflow: auto on the tab panel now works as intended. Clean.

  2. Collapsible linked accounts — New collapsible prop on LinkedAccountBanner, enabled only on the Alerts page. Defaults to collapsed with a connected-count summary, auto-expands on OAuth connect/error. Default behavior unchanged elsewhere (only one call site, and it's the only consumer). Good use of aria-expanded, button semantics are correct.

  3. Test alert button did nothing visible — Three-part fix, all sound:

    • Default event key now uses the first bound key from alertEventKeys[0] with manual.test fallback for unbound canvases. Matches the overlay's behavior (bound canvases ignore unassigned keys).
    • Plays the canvas audio at configured volume using new Audio(). Mirrors OverlayClient.tsx:68-89 almost exactly — same Math.max(0, Math.min(1, volume / 100)) clamp, same .catch() for autoplay restrictions.
    • Replays entrance animations in the editor preview by remounting elements (key change to ${element.id}-${previewAlert?.id}) and setting animationName/animationDuration conditionally on editor.previewAlert. The animationName helper is a direct copy of the one in OverlayClient.tsx:215-220 — same keyframe names, same fallback.
  4. Removed "Live in OBS" pill — Removed from EditorTopBar.tsx and its CSS from globals.css. Grep confirms zero remaining references to canvas-editor-live-badge, canvas-editor-live-dot, or "Live in OBS" in the codebase. Clean removal.

Observations (non-blocking)

  • Duplicated animationName helper — EditorStage.tsx:278-282 and OverlayClient.tsx:215-220 are identical. Could be extracted to a shared util, but given it's 4 lines and the two files serve different purposes (live overlay vs editor preview), the duplication is acceptable. Not worth a refactor here.
  • No test coverage for the new collapsible behavior — The existing LinkedAccountBanner.test.ts only tests getLinkStatus. The collapse/expand logic and connectedCount summary are untested. CI passes because the tests run against pure functions, not the component. Worth adding a test in a follow-up but not blocking.
  • testAlert audio uses new Audio() in the hook — Same pattern as the overlay client, and the .catch() swallows autoplay errors silently, which is the right call for a preview button.

Verification

  • pnpm --filter @multi-stream-alerts/web typecheck — passes
  • pnpm --filter @multi-stream-alerts/web test — 157/157 pass
  • All 11 CI checks on the PR are green

🛶 Approving — ready to merge.

Dock, reviewed PR #107 — approving. Clean PR that addresses four real UX bugs on the Alerts editor page. CI is all green (typecheck, tests 157/157, lint, build, security scans), I re-ran typecheck and tests locally against the PR branch and both pass. ## What the PR does 1. **Canvas jump + non-scrollable Alerts tab** — Adds `min-height: 0` to the left panel's grid shell. Correct fix for the CSS grid `1fr` row overflow issue — without it, tall tab content expands the row past the viewport, pushing the stage down and clipping the list. The existing `overflow: auto` on the tab panel now works as intended. Clean. 2. **Collapsible linked accounts** — New `collapsible` prop on `LinkedAccountBanner`, enabled only on the Alerts page. Defaults to collapsed with a connected-count summary, auto-expands on OAuth connect/error. Default behavior unchanged elsewhere (only one call site, and it's the only consumer). Good use of `aria-expanded`, button semantics are correct. 3. **Test alert button did nothing visible** — Three-part fix, all sound: - Default event key now uses the first bound key from `alertEventKeys[0]` with `manual.test` fallback for unbound canvases. Matches the overlay's behavior (bound canvases ignore unassigned keys). - Plays the canvas audio at configured volume using `new Audio()`. Mirrors `OverlayClient.tsx:68-89` almost exactly — same `Math.max(0, Math.min(1, volume / 100))` clamp, same `.catch()` for autoplay restrictions. - Replays entrance animations in the editor preview by remounting elements (key change to `${element.id}-${previewAlert?.id}`) and setting `animationName`/`animationDuration` conditionally on `editor.previewAlert`. The `animationName` helper is a direct copy of the one in `OverlayClient.tsx:215-220` — same keyframe names, same fallback. 4. **Removed "Live in OBS" pill** — Removed from `EditorTopBar.tsx` and its CSS from `globals.css`. Grep confirms zero remaining references to `canvas-editor-live-badge`, `canvas-editor-live-dot`, or "Live in OBS" in the codebase. Clean removal. ## Observations (non-blocking) - **Duplicated `animationName` helper** — `EditorStage.tsx:278-282` and `OverlayClient.tsx:215-220` are identical. Could be extracted to a shared util, but given it's 4 lines and the two files serve different purposes (live overlay vs editor preview), the duplication is acceptable. Not worth a refactor here. - **No test coverage for the new `collapsible` behavior** — The existing `LinkedAccountBanner.test.ts` only tests `getLinkStatus`. The collapse/expand logic and `connectedCount` summary are untested. CI passes because the tests run against pure functions, not the component. Worth adding a test in a follow-up but not blocking. - **`testAlert` audio uses `new Audio()` in the hook** — Same pattern as the overlay client, and the `.catch()` swallows autoplay errors silently, which is the right call for a preview button. ## Verification - `pnpm --filter @multi-stream-alerts/web typecheck` — passes - `pnpm --filter @multi-stream-alerts/web test` — 157/157 pass - All 11 CI checks on the PR are green 🛶 Approving — ready to merge.
PorteGumi (Migrated from github.com) commented 2026-07-06 14:49:47 -05:00

💡 Suggestion (non-blocking): The collapse/expand logic and connectedCount summary are untested — LinkedAccountBanner.test.ts only covers getLinkStatus. Worth adding component-level tests for the collapsible prop in a follow-up.

💡 **Suggestion (non-blocking):** The collapse/expand logic and `connectedCount` summary are untested — `LinkedAccountBanner.test.ts` only covers `getLinkStatus`. Worth adding component-level tests for the `collapsible` prop in a follow-up.
PorteGumi (Migrated from github.com) commented 2026-07-06 14:49:47 -05:00

💡 Suggestion (non-blocking): The animationName helper here is identical to the one in OverlayClient.tsx:215-220. Consider extracting to a shared canvasUtils.ts in a follow-up if more shared logic accumulates between the overlay and editor preview. Not worth a refactor for 4 lines today.

💡 **Suggestion (non-blocking):** The `animationName` helper here is identical to the one in `OverlayClient.tsx:215-220`. Consider extracting to a shared `canvasUtils.ts` in a follow-up if more shared logic accumulates between the overlay and editor preview. Not worth a refactor for 4 lines today.
Sign in to join this conversation.
No description provided.