fix(www): de-flake self-host Get started dialog E2E - #571
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Get Started dialog now loads at the start of the homepage, uses delegated event handling, and marks readiness on the ChangesGet Started dialog
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The homepage dialog now becomes interactive before users can trigger it, and the updated English and Chinese coverage synchronizes on that readiness state. No current merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR removes the startup race around the marketing homepage’s Get started dialog.
|
| Filename | Overview |
|---|---|
| apps/www/src/components/GetStartedDialog.astro | Replaces deferred per-element listeners with an inline delegated handler and marks the page ready after successful initialization. |
| apps/www/src/components/HomePage.astro | Moves the dialog to the beginning of main content so its inline wiring executes before the visible opener buttons are parsed. |
| apps/www/e2e/marketing-homepage.spec.ts | Waits for the dialog readiness marker before exercising the English and Chinese Get started flows. |
Reviews (2): Last reviewed commit: "fix(www): wire Get started dialog before..." | Re-trigger Greptile
The marketing homepage rendered Get started buttons from SSR immediately, but Astro bundled the dialog opener script as a deferred module at the bottom of the page. Clicks before that module ran were no-ops, which flaked the self-host E2E and could affect fast visitors. Run the dialog wiring as an inline script at the top of main with document-level click delegation, and expose data-get-started-ready for tests to wait on before clicking. Co-authored-by: Elie Steinbock <elie222@users.noreply.github.com>
3e6d917 to
ae7ad9c
Compare
Playwright screenshotsNo new feature frames; gallery is suite-vs-main drift. Open screenshot gallery · Dashboard · CI run Updated for commit |
Why
The
Web E2Emarketing homepage test intermittently fails onmain(~2/3 of recent runs). Playwright clicks the self-host Get started button, but the<dialog>stays hidden becauseshowModal()never runs.Root cause
This was a real page bug, not just a test-timing quirk.
The marketing homepage renders Get started buttons immediately from SSR, but
GetStartedDialog.astrowired them with an Astro<script>block. Astro bundles those as deferred ES modules at the bottom of the page. Until that module executed, clicks on visible buttons were no-ops.The English test runs first in the file and often clicked before the deferred script loaded. The zh variant usually passed because it runs second, after the module had already executed in the same browser session.
What changed
<GetStartedDialog />to the top of<main>and replace the deferred module with an inline script that:showModal()synchronously when wiring completesdata-get-started-readyon<html>once interactivehtml[data-get-started-ready]before clicking, so the test asserts the dialog is actually wired up.Self-host copy assertions (no install-script text like
openssl,POSTGRES_PASSWORD, etc.) are unchanged.How tested
pnpm lintpnpm --filter @rakazo/www checkpnpm --filter @rakazo/www e2e e2e/marketing-homepage.spec.ts— 8 consecutive passes (2 tests each run, 16 total)Summary by CodeRabbit
Bug Fixes
Tests