Skip to content

feat: light mode, Payload tab, and bug fixes#49

Merged
ryanbas21 merged 3 commits into
mainfrom
feat/light-mode
May 13, 2026
Merged

feat: light mode, Payload tab, and bug fixes#49
ryanbas21 merged 3 commits into
mainfrom
feat/light-mode

Conversation

@ryanbas21
Copy link
Copy Markdown
Owner

Summary

  • Light mode with theme toggle (persists via localStorage, survives Elm re-renders)
  • Payload tab in inspector panel showing request/response bodies with JSON tree view
  • Two bug fixes discovered during e2e coverage work
  • 8 new e2e tests covering theme toggle, Payload tab, and import error handling

Bug fixes

Theme toggle invisible on fresh panels: initThemeToggle() runs after Elm.Main.init(), so the MutationObserver missed the initial toolbar render. The toggle only appeared after a subsequent Elm re-render (e.g. when an event arrived). Fixed by eagerly appending the button when the toolbar already exists.

Resize handle intercepting inspector tab clicks: The vertical resize handle (.resize-handle-v) had height: 100% with z-index: 1000, spanning the full viewport including the inspector tab bar. Clicks on tabs positioned near the graph panel edge would hang because the invisible handle intercepted pointer events. Fixed by limiting height to calc(100% - var(--insp-h)).

Test plan

  • 43 Chrome e2e tests pass (0 failures, 0 flaky)
  • 490 unit tests pass
  • Theme toggle visible immediately on fresh panel load
  • Tab switching works across all inspector tabs without hanging
  • Import error banner appears for invalid/malformed JSON

🤖 Generated with Claude Code

Two bugs fixed:

1. Theme toggle never appeared on fresh panels with no stored events.
   initThemeToggle() runs after Elm.Main.init(), so the MutationObserver
   missed the initial toolbar render. Added eager appendChild when the
   toolbar already exists.

2. Vertical resize handle (.resize-handle-v) had height: 100% with
   z-index: 1000, spanning the full viewport including the inspector
   tab bar. This intercepted pointer events on tabs positioned at the
   graph panel edge, causing clicks to hang. Changed to
   calc(100% - var(--insp-h)) so it stops at the inspector boundary.

Also adds e2e tests for theme toggle (4), Payload tab (2), and import
error handling (2).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR — Bug fixes for the theme toggle (eager append on fresh panels) and resize handle (height overlap with inspector tabs), plus 3 new e2e test suites covering import errors, Payload tab, and theme toggle behavior. Two findings in payload-tab.test.ts and one optional CSS suggestion.

Key changes

  • Fix theme toggle invisible on fresh panels — Eager append when .toolbar already exists at init time
  • Fix resize handle intercepting inspector tab clicks — Height changed from 100% to calc(100% - var(--insp-h))
  • Add e2e test: import error handling — 2 test cases for invalid and wrong-schema JSON
  • Add e2e test: Payload tab — 2 test cases for visibility with/without body data
  • Add e2e test: theme toggle — 4 test cases for visibility, switch, persistence, re-render survival

Summary | 5 files | 1 commit | base: mainfeat/light-mode


Eager theme toggle append

Before: initThemeToggle() ran after Elm.Main.init(), so the MutationObserver missed the initial toolbar render. The toggle only appeared after a subsequent Elm re-render.
After: The code also checks if .toolbar already exists and appends immediately.

The parentElement guard in the MutationObserver correctly prevents double-append — the eager append sets btn.parentElement === toolbar before the observer fires, so the guard evaluates to false and skips re-appending.

panel.ts


Resize handle height correction

Before: .resize-handle-v had height: 100%, spanning the full viewport including the inspector tab bar. Tabs near the graph panel edge were unclickable.
After: height: calc(100% - var(--insp-h)) limits the handle to the graph panel area.

panel.css

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run𝕏

Comment thread e2e/tests/payload-tab.test.ts Outdated
Comment thread e2e/tests/payload-tab.test.ts Outdated
Comment thread packages/devtools-ui/src/panel.css Outdated
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented May 13, 2026

Addressed all 3 review threads on PR #49:

  1. payload-tab.test.ts:42 — Renamed test title from "Payload tab does not appear for discovery events" to "Payload tab for discovery events shows only Response Body section"
  2. payload-tab.test.ts:62-69 — Replaced if (isVisible) hedge with direct expect(payloadTab).toBeVisible() for deterministic assertion
  3. panel.css:1136 — Added fallback value var(--insp-h, 220px) for resilience

Changes committed as 922bf5c and pushed to feat/light-mode. All threads replied to and resolved.

Task list (4/4 completed)

Pullfrog  | View workflow run | via Pullfrog𝕏

@ryanbas21
Copy link
Copy Markdown
Owner Author

@pullfrog review this and approve or request further changes

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR — Eager theme toggle append and resize handle CSS fix, plus 8 new e2e tests. Production code changes are correct and well-constrained. One minor test gap: the "only Response Body" claim for GET events is not actually verified.

Key changes

  • Fix theme toggle not appearing on initial panel load — eagerly append the toggle button when .toolbar already exists, preventing the MutationObserver from missing the initial render
  • Fix resize handle intercepting inspector tab clicks — limit vertical handle height to calc(100% - var(--insp-h)) so it doesn't overlap the tab bar
  • Add e2e tests for import error handling — verifies error banner on invalid JSON and wrong-schema JSON, plus panel recoverability
  • Add e2e tests for Payload tab — verifies tab visibility for POST (req+res) and GET (res only) events
  • Add e2e tests for theme toggle — visibility, round-trip switching, localStorage persistence, Elm re-render survival

Summary | 5 files | 2 commits | base: mainfeat/light-mode


Eager theme toggle append (panel.ts)

Before: Theme toggle appeared only after a subsequent Elm re-render (e.g. when an SDK event arrived).
After: Toggle appended synchronously when .toolbar already exists in the DOM.

The eager-append logic is safe: the MutationObserver guard at panel.ts:127 (btn.parentElement !== toolbar) prevents double-append, and the observer continues to handle Elm re-renders that replace the toolbar element.

panel.ts


Resize handle overlap fix (panel.css)

Before: .resize-handle-v at height: 100% spanned the full viewport, intercepting clicks on inspector tabs near the graph panel edge.
After: Height limited to calc(100% - var(--insp-h, 220px)), stopping at the inspector panel boundary.

The 220px fallback matches the :root default for --insp-h. One minor pre-existing note: the handle still extends 32px into the toolbar area (top: 0), but this is unchanged from before and out of scope.

panel.css


New e2e tests

3 new files with 8 total tests covering import error handling, payload tab behavior, and theme toggle lifecycle. Tests follow established patterns (fixture imports, page lifecycle, helper usage). One gap noted inline below.

import-error.test.ts · payload-tab.test.ts · theme-toggle.test.ts

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run𝕏

Comment thread e2e/tests/payload-tab.test.ts
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented May 13, 2026

Addressed the review comment: added assertion verifying Request Body is absent for GET events in e2e/tests/payload-tab.test.ts. Pushed to feat/light-mode as commit a5b2695, replied to comment, and resolved the thread.

Pullfrog  | View workflow run | via Pullfrog𝕏

@ryanbas21
Copy link
Copy Markdown
Owner Author

@pullfrog approve or request changes

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found.

TL;DR — Bug fixes for the theme toggle (eager append on fresh panels) and resize handle (clicks now pass through to inspector tabs), plus 8 new e2e tests across theme toggle, Payload tab, and import error handling.

Key changes

  • Fix theme toggle eager append — The toggle button now appears immediately on fresh panel loads by eagerly appending it when the toolbar already exists after Elm init
  • Fix resize handle overlap — The vertical resize handle height is now limited to calc(100% - var(--insp-h)), preventing it from intercepting clicks on inspector tabs
  • Add e2e tests for theme toggle — 4 tests covering button visibility, toggling behavior, localStorage persistence across reloads, and survival of Elm re-renders
  • Add e2e tests for Payload tab — 2 tests for events with POST (both Request Body and Response Body sections visible) and GET (only Response Body section)
  • Add e2e tests for import error handling — 2 tests for invalid JSON and valid JSON with wrong schema

Summary | 5 files | 3 commits | base: mainfeat/light-mode


Theme toggle eager append

Before: Toggle button only appeared after a subsequent Elm re-render (e.g. when an event arrived)
After: Button is eagerly appended if the toolbar DOM node already exists at initThemeToggle() time

The fix is minimal and correct. The eager append runs synchronously before the MutationObserver callback can fire (microtask ordering), so there's never a double-append. The observer's btn.parentElement !== toolbar guard handles the Elm re-render case where the toolbar element itself may be replaced.

panel.ts

Resize handle overlap fix

Before: .resize-handle-v had height: 100%, intercepting pointer events on inspector tabs near the graph panel edge
After: height: calc(100% - var(--insp-h, 220px)), keeping the handle confined to the graph panel area

The CSS var() fallback (220px) matches the :root default for --insp-h, and the JS that sets --insp-h clamps the value to [80, 600], so the calc expression always produces a sensible height.

panel.css

E2E test coverage

The 8 new tests follow the existing codebase patterns (shared BrowserContext, reloadAndWaitForEvents, panelPage.close() for teardown). Standouts:

  • Theme toggle persistence (full round-trip: click → localStorage → reload → verify data-theme attribute) and Elm re-render survival (inject SDK event → wait for .tl-row → re-check toggle) are the right high-value scenarios.
  • Payload tab correctly distinguishes POST (both sections visible) and GET (only Response Body) — the latter assertion that Request Body is absent is the right edge case to pin.
  • Import error tests cover both JSON.parse failure and Schema.decodeUnknownSync failure paths. Note: these follow the existing decode-error-banner.test.ts pattern of asserting banner visibility without checking message content. This is consistent with current conventions; if the error-reporting surface grows more nuanced in the future, asserting on .err-banner text content would make the tests more precise.

import-error.test.ts · payload-tab.test.ts · theme-toggle.test.ts


Reviewed with correctness and test-integrity lenses. No blocking issues found — the source changes are correct and the test coverage is well-structured.

Pullfrog  | View workflow run𝕏

@ryanbas21 ryanbas21 merged commit 15d9454 into main May 13, 2026
1 check passed
@ryanbas21 ryanbas21 deleted the feat/light-mode branch May 13, 2026 22:31
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