fix(web): settings shows on first load + global DemoProvider#1
Merged
offendingcommit merged 4 commits intomainfrom May 3, 2026
Merged
fix(web): settings shows on first load + global DemoProvider#1offendingcommit merged 4 commits intomainfrom
offendingcommit merged 4 commits intomainfrom
Conversation
Bug 1: On a fresh load with no saved config, RootLayout returned `null` while a useEffect-driven `router.navigate()` fired, leaving a blank screen until the user manually refreshed. Move the redirect into the root route's `beforeLoad` so it happens synchronously during route resolution and the settings form renders on first paint. Bug 2: `DemoProvider` was mounted inside `RootLayout` only on the non-settings branch, so any component reading `useDemo()` outside that branch would throw "useDemoContext must be used within DemoProvider". Hoist `<DemoProvider>` to `main.tsx` so the context is available app-wide. Adds vitest + RTL setup with regression tests for both behaviours.
Settings page was rendering Outlet directly, omitting the Sidebar nav. Adds a playwright e2e test asserting sidebar visibility on both dashboard and settings routes.
- Promote @playwright/test to the workspace catalog
- Add test:e2e turbo task (uncached)
- Add root pnpm test:e2e script
- Vitest scopes to src/**/*.{test,spec} and excludes e2e/
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
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
Two bugs in
packages/web:Bug 1 — Blank screen on first load
__root.tsxcalledloadConfig()synchronously, returnednullif no config existed, and only firedrouter.navigate({ to: '/settings' })from auseEffect. The first paint was empty until the effect ran (and on StrictMode double-invoke + scheduling, often required a manual refresh to surface the settings page).Fix: moved the redirect into the root route's
beforeLoad, so the redirect resolves synchronously during route loading and the settings form is the first thing rendered.Bug 2 —
DemoProvidernot globally available<DemoProvider>was mounted insideRootLayoutafter theif (!config) return nullearly return and only on the non-settings branch. Any consumer ofuseDemo()outside that branch would throwuseDemoContext must be used within DemoProvider.Fix: hoisted
<DemoProvider>up intomain.tsxso the context wraps the entire app (settings route included).Tests
Added
packages/web/vitest.config.ts, jsdom setup, andsrc/test/app.test.tsx:first load with no config > renders the settings form on first paint when no config exists— fails before the fix (empty body), passes after.Sidebar/useDemo availability across routes > does not throw when a useDemo consumer mounts alongside the routed app— guards the global-provider wiring.Confirmed both tests fail for the right reason on the unfixed code, then pass after the fix.
Test plan
pnpm --filter @openconcho/web testpnpm --filter @openconcho/web typecheckpnpm --filter @openconcho/web build/→ settings form visible immediately, no refresh needed./settings, nouseDemoContextconsole error.Notes
pnpm lintis broken onmaindue to a pre-existing biome config / version mismatch (biome.jsonuses 2.x keys, installed biome is 1.9.4). Not addressed here to keep the PR scoped — flag for follow-up.