Skip to content

fix(web): keep HTML prototype selection rings visible - #23745

Open
stevan-borus wants to merge 3 commits into
pingdotgg:mainfrom
stevan-borus:fix/prototype-focus-clipping
Open

stevan-borus wants to merge 3 commits into
pingdotgg:mainfrom
stevan-borus:fix/prototype-focus-clipping

Conversation

@stevan-borus

Copy link
Copy Markdown
Contributor

Problem

Selection rings on generated HTML prototypes are cut off when the first or last card touches the iframe edge. A card with a 3px outline and 2px outline offset paints 5px outside the page, while the fitted frame only includes the document's layout box.

Change

Give newly generated HTML pages an overridable 8px gutter, with border-box sizing so fluid content still fits the frame. Update the agent layout guidance to account for that gutter. The existing inset keyboard-focus rule stays unchanged.

Add an integration test using the real HtmlRenderDocument sandboxed iframe and injected bootstrap. It selects both edge cards and checks their ring bounds, fitted height, and horizontal overflow.

Scope and approval

This is a small, focused fix for an obvious visual clipping bug in the existing HTML renderer. It adds no new workflow or setting. The regression test was explicitly requested by the developer.

Verification

  • TDD: the iframe regression fails on the original CSS with expected -5 to be greater than or equal to 0, then passes with the gutter.
  • vp test run apps/web/src/components/files/BrowserDocumentFrame.browser.test.tsx packages/shared/src/htmlRender.test.ts: 2 files, 12 tests passed.
  • vp run check:changed upstream/main: formatting, lint, knip, typechecks for 28 affected packages, and both test files passed.
  • Independent GPT-6.1-Sol review of dc667126e7: no findings.
  • Dev-client verification at desktop and 360px browser widths: first/last selected rings have 3px clearance, the document and fitted frame both measure 199px, and no horizontal overflow occurs. The client used a disposable synthetic thread and HTML attachment. Native mobile clients and keyboard interaction were not separately exercised.

Fresh prototype rendered with the production headless screenshot helper, using the same source before and after. The original report image was not used. Integrated app screenshot capture timed out, so these captures show the rendered document itself.

Before, clipped at the left and bottom:

Fresh prototype before the fix

After, complete selection ring:

Fresh prototype after the fix

Notes

The default gutter applies when pages are generated. Previously stored HTML snapshots retain their embedded styles and need to be regenerated. Page-authored CSS can override the gutter.

Implementation: GPT-6.1-Sol via Codex in T3 Code; reviews by GPT-6.1-Sol and GPT-6-Astra via Codex.

Generated HTML pages start flush against the iframe viewport, so selected cards can paint their rings outside the visible page. Reserve an overridable 8px gutter and include it in normal document sizing, with an iframe regression covering both edge cards.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 22, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The fix is narrowly scoped and includes an iframe regression test, but it changes the default CSS and layout guidance applied to newly generated HTML pages. That product-default behavior change warrants explicit review despite the small diff.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between becf853 and 8a03008.

Files selected for processing: 2
  • apps/web/src/components/files/BrowserDocumentFrame.browser.test.tsx
  • packages/shared/src/htmlRender.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

The shared HTML bootstrap now applies an overridable 8px root gutter with border-box sizing. A browser test checks that the first and last selectable-card outlines fit within a fitted iframe viewport and that the document has no horizontal overflow.

Changes: HTML render frame bounds

Layer / File(s) Summary
Root gutter contract and CSS
packages/shared/src/htmlRender.ts
The layout guide documents an overridable 8px HTML gutter. The base CSS applies border-box sizing and 8px padding to the root element.
Fitted iframe bounds test
apps/web/src/components/files/BrowserDocumentFrame.browser.test.tsx
The browser test checks the first and last selected-card outlines against the fitted iframe viewport and verifies that the document has no horizontal overflow.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#23864: The changes adjust generated page sizing and test page-to-viewport height and overflow. They do not implement the issue’s proposed scroll-suppression fix.

Suggested reviewers: t3dotgg, maria-rcks

Merge Risk: Minimal · up to 8a030

No actionable issue remains identified for the HTML selection-ring gutter; the change is mergeable after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "fix(web): keep HTML prototype selection rings visible" follows conventional commit format and names the primary change: stopping selection rings on generated HTML prototypes from being clip…
Description check Passed The description follows the template's main sections. Problem gives the clipping cause and reproduction detail: a 3px outline with 2px offset paints 5px outside the fitted frame. Change explains the 8…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix(web): keep HTML prototype selection rings visible" follows conventional commit format and names the primary change: stopping selection rings on generated HTML prototypes from being clipped at the iframe edge. The diff adds an 8px html gutter in the shared renderer CSS, updates the layout guide, and adds a browser test for edge cards, so the title matches the changeset.

Full details: Description check

Explanation

The description follows the template's main sections. Problem gives the clipping cause and reproduction detail: a 3px outline with 2px offset paints 5px outside the fitted frame. Change explains the 8px overridable gutter, border-box sizing, the updated layout guidance, and the integration test. Scope and approval states this is a small, focused fix and that the regression test was requested. Verification lists the TDD failure, the focused test run, check results, and before/after screenshots. It also states what was not checked: native mobile clients and keyboard interaction. Minor gaps: the Scope section does not link a triaged issue or maintainer approval comment, and the template asks for an explicit approval reference for small fixes, which the author addresses only by explanation. The description also includes a model/harness attribution line, which the template requests.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

maria-rcks commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

thanks @stevan-borus. the overridable root gutter and border-box sizing fix the clipping without changing keyboard focus. i kept your commit and merged current main (dd5b5401e1), combined your regression with main's existing iframe test, and reduced the guidance update to one sentence. the final diff is two files, 109 additions and 1 deletion.

verification on 5277dcf4c04ea1ecd5afc8b689371e78425c8c93:

  • reproduced the regression against main's css: expected -5 to be greater than or equal to 0.
  • blacksmith vp run check:changed origin/main passed format, lint, both knip passes, 29 affected-package typechecks, and all 13 focused tests.
  • published matching HTML through HtmlRender.publish and loaded its signed attachment in the real app. clicking the first and last cards leaves 3px clearance at all four frame edges. desktop/dark and phone/light fit the 256px page without horizontal overflow.
  • keyboard focus remains 2px with a -2px offset; authored html{padding:24px} overrides the gutter through a theme update. root width:100% does not overflow. fullscreen open/close passed.
  • two independent read-only reviews approved this exact head with no findings.
  • ci passed build/typecheck, lint, package tests, all eight server shards, release smoke, and native fingerprint. after one rerun, the web suite has 4,591/4,593 passing tests; two unchanged Word-preview tests time out in AttachmentFilePreview.browser.test.tsx (default template styles and binary package properties). current main also fails in that same file, in other Word-preview cases. takeover ci remains blocked on that existing viewer suite. no unrelated viewer changes were added.
  • coderabbit reported a usage limit rather than findings. both independent reviews approved the exact head. macroscope still calls for human approval of this product default.
  • uploaded images were downloaded and byte-checked; the uploaded video played and sought correctly in chrome. cafe renders external media as links; inline media rendering there is unavailable. task dev processes and the blacksmith worker are stopped.
  • unverified: native mobile, electron, and a provider-cli-generated turn. the app captures use a disposable tool-output fixture. existing stored pages retain their original styles until regenerated.

main before, desktop:

desktop before: selection ring clipped at the iframe edges

updated branch, desktop:

desktop after: complete selection ring inside the fitted iframe

main before, phone width:

phone before: selection ring clipped at the iframe edges

updated branch, phone width:

phone after: complete selection ring inside the fitted iframe

first/last selection and inset keyboard focus in the app:

selecting edge cards and moving keyboard focus

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

merged current main (becf853defb6) cleanly and pushed 8a030080bd86229467ac8851d6e4567535c6e94d to the existing fork branch without force. the diff remains two files, 109 additions and 1 deletion. no source fixes or unrelated viewer changes were needed.

blacksmith vp run check:changed github/main passed format, lint, both knip passes, all 30 affected-package typechecks, and all 13 focused tests. chrome with the actual HtmlRenderDocument component also passed first/last card selection: 3px clearance at the frame edges and no horizontal overflow. both independent read-only reviews approved this exact head with no findings. prior before/after evidence remains applicable because the implementation is unchanged.

the old failed CI log contains two Word-preview timeouts, outside this diff. main's latest CI passed. fresh CI is pending on the merged head. native mobile, electron, and a provider-cli-generated turn were not exercised this pass.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants