Feat/fix blank page bug 3180#3188
Conversation
📝 WalkthroughWalkthrough
ChangesEmpty page cleanup fix and E2E round-trip test
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/libs/resume/move-item.ts`:
- Around line 164-173: The cleanup in move-item.ts leaves a stale
customSectionId in page.main when a custom section is removed, so update the
page’s main array before deciding whether to delete the page. In the removal
block inside the moveItem/custom section handling, use the found page from
draft.metadata.layout.pages to remove customSectionId from its main list, then
only splice the page if it becomes empty. This should keep draft.customSections,
draft.metadata.layout.pages, and the section-item render path consistent.
In `@tests/e2e/playwright.config.ts`:
- Around line 28-31: The Playwright webServer is starting the full workspace dev
graph via the npm run dev script instead of just the app under test, which can
pull in unrelated services and slow or block CI. Update the webServer command in
playwright.config.ts to use the dedicated web app script that serves the UI on
http://localhost:3000, keeping the rest of the Playwright configuration
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d7dbed5b-e723-4d0b-a06e-03dc973b5176
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/web/src/libs/resume/move-item.tspackage.jsontests/e2e/playwright.config.tstests/e2e/specs/data/test-empty-page.jsontests/e2e/specs/test-empty-page.spec.ts
| if (customSection.items.length === 0) { | ||
| const sectionIndex = draft.customSections.indexOf(customSection); | ||
| draft.customSections.splice(sectionIndex, 1); | ||
|
|
||
| const customSectionPageIndex = draft.metadata.layout.pages.findIndex((page) => | ||
| page.main.includes(customSectionId), | ||
| ); | ||
| if (draft.metadata.layout.pages[customSectionPageIndex].main.length === 1) { | ||
| draft.metadata.layout.pages.splice(customSectionPageIndex, 1); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove the section ID from page.main before deciding whether to delete the page.
Line 168 only finds the page, but Lines 171-172 delete the page only when main.length === 1. On any page with other sections, this leaves a dangling customSectionId in draft.metadata.layout.pages[*].main after the custom section has already been removed from draft.customSections. The move/render path in apps/web/src/routes/builder/$resumeId/-sidebar/left/shared/section-item.tsx now sees a layout entry that points at nothing.
Suggested fix
if (customSection.items.length === 0) {
const sectionIndex = draft.customSections.indexOf(customSection);
draft.customSections.splice(sectionIndex, 1);
const customSectionPageIndex = draft.metadata.layout.pages.findIndex((page) =>
page.main.includes(customSectionId),
);
- if (draft.metadata.layout.pages[customSectionPageIndex].main.length === 1) {
- draft.metadata.layout.pages.splice(customSectionPageIndex, 1);
+ if (customSectionPageIndex !== -1) {
+ const page = draft.metadata.layout.pages[customSectionPageIndex];
+ page.main = page.main.filter((sectionId) => sectionId !== customSectionId);
+
+ if (page.main.length === 0) {
+ draft.metadata.layout.pages.splice(customSectionPageIndex, 1);
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (customSection.items.length === 0) { | |
| const sectionIndex = draft.customSections.indexOf(customSection); | |
| draft.customSections.splice(sectionIndex, 1); | |
| const customSectionPageIndex = draft.metadata.layout.pages.findIndex((page) => | |
| page.main.includes(customSectionId), | |
| ); | |
| if (draft.metadata.layout.pages[customSectionPageIndex].main.length === 1) { | |
| draft.metadata.layout.pages.splice(customSectionPageIndex, 1); | |
| } | |
| if (customSection.items.length === 0) { | |
| const sectionIndex = draft.customSections.indexOf(customSection); | |
| draft.customSections.splice(sectionIndex, 1); | |
| const customSectionPageIndex = draft.metadata.layout.pages.findIndex((page) => | |
| page.main.includes(customSectionId), | |
| ); | |
| if (customSectionPageIndex !== -1) { | |
| const page = draft.metadata.layout.pages[customSectionPageIndex]; | |
| page.main = page.main.filter((sectionId) => sectionId !== customSectionId); | |
| if (page.main.length === 0) { | |
| draft.metadata.layout.pages.splice(customSectionPageIndex, 1); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/libs/resume/move-item.ts` around lines 164 - 173, The cleanup in
move-item.ts leaves a stale customSectionId in page.main when a custom section
is removed, so update the page’s main array before deciding whether to delete
the page. In the removal block inside the moveItem/custom section handling, use
the found page from draft.metadata.layout.pages to remove customSectionId from
its main list, then only splice the page if it becomes empty. This should keep
draft.customSections, draft.metadata.layout.pages, and the section-item render
path consistent.
| webServer: { | ||
| command: "npm run dev", | ||
| url: "http://localhost:3000", | ||
| reuseExistingServer: !process.env.CI, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Start only the web app here, not the whole workspace dev graph.
Line 29 currently shells out to npm run dev, but package.json maps that to turbo run dev. That is much broader than the single app this config probes on http://localhost:3000 and can make CI wait on unrelated services instead of the builder UI. Point this at the dedicated web script instead.
Suggested fix
webServer: {
- command: "npm run dev",
+ command: "pnpm dev:web",
url: "http://localhost:3000",
reuseExistingServer: !process.env.CI,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| webServer: { | |
| command: "npm run dev", | |
| url: "http://localhost:3000", | |
| reuseExistingServer: !process.env.CI, | |
| webServer: { | |
| command: "pnpm dev:web", | |
| url: "http://localhost:3000", | |
| reuseExistingServer: !process.env.CI, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/playwright.config.ts` around lines 28 - 31, The Playwright
webServer is starting the full workspace dev graph via the npm run dev script
instead of just the app under test, which can pull in unrelated services and
slow or block CI. Update the webServer command in playwright.config.ts to use
the dedicated web app script that serves the UI on http://localhost:3000,
keeping the rest of the Playwright configuration unchanged.
1 participant
Fix and e2e test for issue #3180 .
ps: I'm completely new in typescript, so please don't hesitate to tell me my errors.
Thanks.
Summary by CodeRabbit
Bug Fixes
Tests