Repository navigation
Keep the whole planner setup within its panes at narrow widths and enlarged text - #3258
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…t. At 768, 842 and 880px the planner canvas no longer scrolls sideways. Everything was tested in headless Chromium with a stubbed backend, not in the signed native app; the 842px case only stands in for the native workspace width. Reproduction (before any change): I stubbed the branch lookup as successful, so the header error from #3253 never appears. Viewport height was 900px. The canvas that scrolls is the setup wizard's `flex-1 flex flex-col md:flex-row min-h-0 overflow-auto` element, not the document. The document itself never overflowed. | Viewport | New plan canvas, before → after | Edit plan canvas, before → after | |---|---|---| | 768px | 768/833 → 768/768 | 768/841 → 768/768 | | 842px | 842/881 → 842/842 | 842/889 → 842/842 | | 880px | 880/906 → 880/880 | 880/914 → 880/880 | | 1280px, 1390px | no overflow either way | no overflow either way | Cause: In the full layout, the header row in `ContextLevelSlider.tsx` didn't wrap, and the estimate was `whitespace-nowrap` (180.6px wide). The right pane is 35% wide, so at 768px it has about 228px for content. The title group and the estimate couldn't fit side by side, so the estimate ran past the pane. At 1280/1390px it only fitted because the "Context Scope" label was squeezed onto two lines. Fix: only `ContextLevelSlider.tsx`, full layout. Budget calculations, slider steps, callbacks, labels and the compact mobile layout are unchanged. - The header row and the title group can now wrap, so the estimate drops under the title when the pane is narrow. - The estimate breaks only after the token budget ("≤490k tokens ·" then "~1-2m scan"). A no-break space keeps the "·" with "tokens", and each half wraps internally only if it can't fit on its own. - The Focused / Expanded / Full Scan shortcut row can wrap too. With 200% text at 1280px, "Full Scan" was running past the pane. - At 1280/1390px the row is the same 40px tall as before, but the label stays on one line and the estimate sits under it. Tests: - New `e2e/planner-scope-estimate.pw.ts`: covers new and edit plans at 320, 390, 768, 842, 880, 1280 and 1390px, plus 150% and 200% text. - It checks every horizontal scroll container up to the document, and that nothing in the scope block reaches past the pane. - It checks the full estimate, percentage, label, slider and shortcuts are readable and on screen, and steps the slider from the keyboard at each width. - It checks the typed prompt and the slider value survive resizing (including switching between the compact and full layouts). - It fails on any write except edit mode's existing autosave of its prompt; new mode sends nothing. - New mode keeps an empty prompt, because typing there creates a draft after a second. - Negative control: with the original component restored, all 6 new tests fail (for example `canvas 768/833`). With the fix all 6 pass. - New unit test in `ContextLevelSlider.test.tsx`. - Final run: `npx playwright test e2e/planner-scope-estimate.pw.ts e2e/planner-studio.pw.ts e2e/planner-studio-mobile.pw.ts` gives 42 passed. Vitest (`ContextLevelSlider`, `SetupWizardRightPane`, `SetupWizard`): 19 passed. `eslint --max-warnings 0` and `tsc --noEmit` are clean. Still overflowing with enlarged text, outside this component: the canvas still measures 768/772 at 150%, 768/862 at 200% and 800/883 at 1280px/200%. That comes from other controls: the "(optional)" label in Additional Context Repositories, the docked Generate Plan label, and the mobile settings chevron at 390px/200%. The left-pane header is also clipped at 200%. These are not in the scope estimate, so I didn't touch them. For enlarged text the tests check only that the scope block stays inside the pane, not the whole canvas. I kept to `ContextLevelSlider.tsx`, its test, and the new spec, so none of the files touched by PR3254, PR3255 or PR3256 are changed. Previews include before/after screenshots at 768px and of the 1280px pane, plus 880px and 200%-text views after the fix.
|
/merge Please integrate current main124acbe17451373ebe984481956577ccd0c34aea before final independent validation and Astra review. Preserve the focused scope-estimate implementation and tests. Keep this a bounded integration with relevant static checks; operator will run independent browser suites on the stable head. The remaining enlarged-text overflow outside ContextLevelSlider is a separate release-validation finding and will not be counted as fixed by this PR. Do not merge this PR into main or publish. |
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
…ude-opus-5-5-keep-planner-scope-estima-20261019-0527-fyp
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationMerge-ready within the reviewed scope, conditional on pending checks completing successfully. The change is focused and preserves the estimate calculation and slider behavior while allowing constrained layouts to wrap. ✅ Targeted wrapping — The full header and shortcut row can wrap; separate estimate segments preserve a preferred break between token budget and scan time. ✅ Relevant regression coverage — Browser tests cover new and existing plans, medium widths, enlarged text, keyboard interaction, prompt preservation, and unexpected writes. This is a static review of the supplied complete diff and surrounding excerpts; no commands or tests were run. The original issue description is truncated, so omitted acceptance criteria could not be assessed. The supplied check status reports no failures and six pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Assert estimates after keyboard changesIn ScoreThe implementation addresses the visible overflow objective with a localized change and relevant regression coverage. No introduced defect was identified; runtime validation remains subject to the pending checks and independent browser runs. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
Complete the remaining whole-planner layout acceptance from #3257 and staged #3259 in this existing PR before merge. There is no other implementation writer on3258. The scope estimate fix itself has independently passed all165 cases (55 per full Chromium/headless-shell/WebKit), and Astra9/10 found no blocker at401b341425d77bf7d1f682afe199c5b407739506. Keep that fix. The remaining whole-canvas acceptance cannot be counted as passed merely because the scope span fits. I independently reproduced the residual against this exact production build, using source-owned API fixtures, successful branch lookup, no credentials/real writes, full Chromium151.0.7922.34, both new and existing plans, height900 and actual150/200% root text. All eight cases have horizontal overflow although every scope estimate fits its pane:
The independent old/new scope suite was not changed or weakened to obtain these measurements. Source and Git tree remained clean after the separate diagnostic. I directly inspected the enlarged production screenshots; phase labels and the Break plan/footer controls visibly extend beneath the adjacent pane. Original component fix is valid; the surrounding flow is still incomplete. Please fix the actual remaining planner header/left-pane/footer/compact-settings/right-pane context-label overflow while preserving the scoped estimate fix. Use #3259 acceptance as detail: whole canvas and document fit at320/390,768/842/880 and1280/1390 with ordinary150/200% text, full labels/controls readable and reachable, owned prompt/context/repo values preserved, no unintended writes. Keep calculations, generation, permissions and draft behavior unchanged. Do not hide/clip content or reduce user text. Do not change the diff PR3254 or creation dialog3256. The former3255 header error fix is already merged, so its files are no longer an overlapping writer; preserve its below-header error handling while fixing enlarged header flow if required. Add meaningful negative controls proving the current401 layout fails and the corrected whole-canvas layout passes, including mobile390/200%; remove the test exemption that avoids whole-canvas assertions for enlarged text once these causes are fixed. Resolve model metadata in any new footer-specific fixture before treating a loading spinner as a permanent product layout defect. Paths retained locally: |
|
✅ Applied the requested follow-up changes in commit 4758b4c Summary of ChangesThe planner setup canvas now fits horizontally in every case you listed. In my headless Chromium runs (stubbed backend, no credentials), new and existing plans showed no sideways scroll at 320, 390, 768, 842, 880, 1280 and 1390px with 100%, 150% and 200% text. All labels and controls are whole and on screen, and your scope-estimate fix is unchanged. What was overflowing besides the scope estimate:
The 65/35 split between the panes is unchanged, and the test checks that. Nothing is hidden and no text is made smaller. Calculations, generation, permissions, drafts and the below-header error handling from #3255 are untouched, and no diff (#3254) or creation-dialog (#3256) files were changed. Tests:
Proof the 401 layout fails: I built 401b341 with one layout-neutral test id added so the probe could find the left pane, and ran the new specs against it:
Final run:
Nothing was run in WebKit, headless-shell or the native app. Two things outside the planner canvas, which I left alone: the bottom navigation cuts "Dashboard" at 320px even at 100% text, and the top-bar search box is squeezed at 200%. The previews show before and after at 768px and 1280px with 200% text, and at 390px on a phone. Visual previewAfter: existing plan at 1280px, 200% text
The typed prompt is kept. propr / main sit above the wrapped phase pill, and Break plan, the full model name, Export and Generate Plan are whole inside the prompt box. Canvas measures 800/800. Original unavailable: managed storage is unavailable or the upload has expired. After: phone at 390px, 200% text
The full model name wraps inside its picker and the context repos picker moves under its label. Canvas measures 390/390. Original unavailable: managed storage is unavailable or the upload has expired. After: new plan at 768px, 200% text
The phases wrap inside their pill under propr/main, and Break plan, the full model name, Export and Generate wrap onto lines inside the prompt box. Canvas measures 768/768; the 65/35 pane split is unchanged. Original unavailable: managed storage is unavailable or the upload has expired. Before: existing plan at 1280px, 200% text
Build 401b341, existing plan with its typed prompt. The phase pill runs under the context pane and pushes the repository name out of view, and the Granular pill and Model control are cut off at the prompt box's edge. Canvas measures 800/883. Original unavailable: managed storage is unavailable or the upload has expired. Before: phone at 390px, 200% text
Build 401b341. The model name is cut to 'Claude …' and the 'None indexed' picker runs past the screen. Canvas measures 390/404. Original unavailable: managed storage is unavailable or the upload has expired. Before: new plan at 768px, 200% text
Build 401b341. The phase pill runs under the context pane ('3 Execu…'), the repository name is gone, and the Break plan / Model row is cut off at the pane edge. Canvas measures 768/862. Original unavailable: managed storage is unavailable or the upload has expired. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/merge After the current implementation follow-up6261145049 finishes, integrate latest main, now including the independently reviewed/tested diff fix756dabadcd22ec1ec072f89abeef04c51f336c26 (PR3254). Preserve the current layout correction and all its regressions. This is a new main update since the prior integration, and must not overlap the active writer. Keep integration bounded with relevant static checks; operator will independently test the final stable combined head and request fresh Astra review. Do not merge this PR into main or publish. |
…e you listed. In my headless Chromium runs (stubbed backend, no credentials), new and existing plans showed no sideways scroll at 320, 390, 768, 842, 880, 1280 and 1390px with 100%, 150% and 200% text. All labels and controls are whole and on screen, and your scope-estimate fix is unchanged. The planner setup canvas now fits horizontally in every case you listed. In my headless Chromium runs (stubbed backend, no credentials), new and existing plans showed no sideways scroll at 320, 390, 768, 842, 880, 1280 and 1390px with 100%, 150% and 200% text. All labels and controls are whole and on screen, and your scope-estimate fix is unchanged. **What was overflowing besides the scope estimate:** - **Header:** the phase pill couldn't wrap, so with enlarged text it ran under the right pane and pushed the repo name out of view. The repo and branch now take the first line and the pill wraps under them. The wrapping is opt-in (`wrap` prop on `StudioPhaseSwitcher`), so the other studio headers render exactly as before. When I first changed the pill for every header, the execution-header test at 768px failed, which is why it's opt-in. - **Footer:** the row was forced onto one line. The model picker got squeezed and the Granular pill sat inside a hidden sideways scroller. This was already broken at normal text size: at 1280px/100% the 401 build cut the model name to 61 of its 144px. Break plan, model and Export/Generate now move to new lines when they don't fit. The model name wraps instead of being cut off, and the Generate label can wrap. - **Right pane:** "(optional)" now flows with its "Additional Context Repositories" title, and the long word breaks rather than running past the pane. - **Phone settings:** the "None indexed" picker drops under its "Context repos" label. The settings rows can wrap. The `propr/main` repo pill wraps before `/main` instead of being cut off. - **Context Refresh tooltip** (hover only): kept within the screen width. At 320/200% it used to open off the left edge. The 65/35 split between the panes is unchanged, and the test checks that. Nothing is hidden and no text is made smaller. Calculations, generation, permissions, drafts and the below-header error handling from #3255 are untouched, and no diff (#3254) or creation-dialog (#3256) files were changed. **Tests:** - **New `e2e/planner-whole-canvas.pw.ts`:** covers both modes at 100/150/200% across all seven widths. At each width it checks: - nothing in the document scrolls sideways. - no visible element reaches past its pane or the screen, and no text is cut short. - every label and control can be scrolled fully into view. - the prompt is kept and nothing is written. - **Model name:** each run waits for "Claude Opus 5.5 (Default)" to appear before measuring, so a loading spinner never counts as a layout result. - **Measuring after resize:** each width is measured only after the layout has switched between phone and desktop. Without that wait, the probe picked up a half-switched layout. - **Probe self-test:** a separate test injects an element too wide for its pane, a cut-off label and a sideways scroller, and checks that the probe reports all three. - **Exemption removed:** `planner-scope-estimate.pw.ts` now checks the whole canvas up to the document with enlarged text too. The shared setup moved to `e2e/planner-setup.fixture.ts`. **Proof the 401 layout fails:** I built 401b341 with one layout-neutral test id added so the probe could find the left pane, and ran the new specs against it: - All 6 whole-canvas layout tests failed, and so did all 4 enlarged-text scope tests. That includes the 390/200% phone case, which measured 390/404. - Measured canvas widths matched yours: 768/772 at 150%, 768/862 at 200%, 800/883 at 1280px/200%. With the fix, all 42 cases have no sideways scroll at all. **Final run:** - The 9 planner Playwright specs: 70 passed. - Vitest for `TaskPlanner` and `RepositorySelector`: 297 passed. - `tsc --noEmit` and `eslint --max-warnings 0` are clean. Nothing was run in WebKit, headless-shell or the native app. Two things outside the planner canvas, which I left alone: the bottom navigation cuts "Dashboard" at 320px even at 100% text, and the top-bar search box is squeezed at 200%. The previews show before and after at 768px and 1280px with 200% text, and at 390px on a phone. PR: #3258 Comment by: @integry (ID: 6261145049) Model: Claude Opus 5.5
|
🔀 Auto-merged No conflicts were found — the merge was verified by an AI agent. 🤖 Verification Details
System-triggered merge conflict resolution |
…ude-opus-5-5-keep-planner-scope-estima-20261019-0527-fyp
|
/review gpt-6-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — gpt-6-astraOverall EvaluationThe PR addresses scope-estimate overflow and extends wrapping to the planner header, footer, model selector, and repository controls. The changes are focused on layout and preserve the existing calculations and callbacks. It is merge-ready within the reviewed scope, conditional on the pending checks completing successfully. ✅ Preserved scope semantics — ✅ Broader overflow validation — The new browser specs check ancestor scroll containers, pane boundaries, clipped content, control reachability, and prompt preservation across viewport sizes and enlarged text. The injected negative controls explicitly exercise the whole-canvas detector. ✅ Scoped component options — Phase and model-label wrapping are enabled explicitly for setup, preserving their default behavior for other consumers. This assessment is a static review of the supplied diff and surrounding context. No commands or tests were run. The supplied current-head status reports no failures and six pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S2: 🟢 Cover populated repository controlsThe whole-canvas fixtures exercise the “None indexed” state, leaving expanded repository content, selected chips, and open menus outside this regression coverage. An optional follow-up could exercise those states with long repository and branch names at enlarged text sizes. This would strengthen confidence in the broader layout claim; the supplied diff does not establish a newly introduced defect in those states. S3: 🟢 Check control obstruction
ScoreThe layout changes are coherent and supported by substantial regression coverage, including negative controls. No PR-introduced blocker is established; final merge readiness remains conditional on the pending checks. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
integry
changed the title
Planner setup could scroll sideways or clip labels at medium widths and with enlarged text. This change wraps the scope estimate, phase indicator, repository/branch header, model selector, generation controls, and context-repository labels within their existing panes. The 65/35 split, scope calculations, permissions, and planning behavior stay intact.
Validation at
11be88472d62fa8f6aa87b12dc8d44d62b117825:Browser tests use API fixtures. Signed native Linux and physical-phone testing remain release gates.
Closes #3257.
Closes #3259.