fix(web): preserve share export layout fidelity - #1277
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Minor] Landscape phones are classified as desktop exports at 640px, so they no longer retain the fixed 960px / 2x mobile output. Evidence:
web/src/components/AssistantChat/ShareTurnDialog.tsx:541. - [Minor] Every attachment set with 2+ images is forced into one non-wrapping row, making 3+ image exports progressively unreadable. Evidence:
web/src/components/AssistantChat/ShareTurnDialog.tsx:739.
Summary
Review mode: initial
Two layout regressions found in the new responsive export behavior.
Testing
Not run locally (automation security constraint: PR code treated as untrusted). GitHub test check passed. Missing targeted coverage: 844×390 landscape-mobile export width and 3+/5-image attachment layout.
HAPI Bot
| const preserveSourceLayout = Boolean( | ||
| props.sourceContentWidth | ||
| && props.sourceContentWidth > 0 | ||
| && window.matchMedia('(min-width: 640px)').matches |
There was a problem hiding this comment.
[MINOR] Keep landscape phones on the fixed mobile export path
This treats every viewport at least 640px wide as desktop. HAPI's responsive config uses 920px specifically to stay clear of phone-landscape widths (web/tailwind.config.ts:7). On an 844×390 phone, the measured source width is below 960px, so this branch produces an export narrower than the promised fixed 1920px output; the new test covers only 390px portrait.
Suggested fix:
const preserveSourceLayout = Boolean(
props.sourceContentWidth
&& props.sourceContentWidth > 0
&& window.matchMedia('(min-width: 920px)').matches
)Also add an 844×390 case asserting a 1920px PNG width.
| .hapi-share-preview-root .hapi-share-media-grid:not([data-hapi-image-count="1"]) > button > img { | ||
| width: 100% !important; | ||
| min-width: 0 !important; | ||
| flex-wrap: nowrap !important; |
There was a problem hiding this comment.
[MINOR] Do not force every multi-image set into one row
:not([data-hapi-image-count="1"]) matches 2, 3, 4, ... images, and MessageAttachments has no image-count cap. With nowrap plus shrinking buttons, every additional image is squeezed into the same 720px preview/export row; 3+ attachments quickly become too small to read. The fixture exercises only two images.
Suggested fix:
.hapi-share-preview-root .hapi-share-media-grid[data-hapi-image-count="2"] {
flex-wrap: nowrap !important;
}
.hapi-share-preview-root .hapi-share-media-grid:not([data-hapi-image-count="1"]):not([data-hapi-image-count="2"]) {
flex-wrap: wrap !important;
}Add a 3- or 5-image fixture that verifies wrapping and a usable minimum image width.
There was a problem hiding this comment.
Findings
- [Minor] Touch-capable desktop devices lose source-width exports —
navigator.maxTouchPoints > 0is true on touchscreen laptops and convertibles even at desktop widths, so the new branch forces their export back to the fixed 960px layout instead of preserving the source conversation width. Evidenceweb/src/components/AssistantChat/ShareTurnDialog.tsx:538.
Suggested fix:const preserveSourceLayout = Boolean( props.sourceContentWidth && props.sourceContentWidth > 0 && window.matchMedia('(min-width: 920px)').matches )
Summary
Review mode: follow-up after new commits
One remaining device-classification regression. The previous landscape-phone and 3+ attachment findings are addressed by the new commits.
Testing
Not run (automation security constraint: PR code treated as untrusted). GitHub checks were pending during review. Missing targeted coverage: a wide desktop viewport with maxTouchPoints > 0.
HAPI Bot
| naturalHeight: number | ||
| } | null>(null) | ||
| const showNativeShareButton = true | ||
| const isTouchDevice = navigator.maxTouchPoints > 0 |
There was a problem hiding this comment.
[MINOR] Do not use touch capability as the mobile boundary
A wide Windows touchscreen laptop or convertible reports navigator.maxTouchPoints > 0, so isTouchDevice becomes true and exportWidth is forced to 960px even on a 1440px desktop viewport. That directly defeats this PR's source-width desktop export. The repo already uses 920px as the phone-landscape-safe split boundary (web/tailwind.config.ts:7).
Suggested fix:
const preserveSourceLayout = Boolean(
props.sourceContentWidth
&& props.sourceContentWidth > 0
&& window.matchMedia('(min-width: 920px)').matches
)Keep the 844×390 test and add a 1440px case with maxTouchPoints = 5 that still expects the 2240px source-width export.
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the latest full diff. The prior touch-capable desktop export regression is addressed. Residual risk: the visual-fidelity E2E coverage checks output dimensions and selected computed styles, but does not pixel-compare the exported PNG against the source layout.
Testing
- Not run (automation security constraint: PR code treated as untrusted).
- GitHub
testandpr-reviewchecks were pending at review time.
HAPI Bot
2 participants
Summary
Improve share-image layout fidelity across desktop and mobile exports.
Implementation
Desktop exports now capture the source message-container width and use it to calculate the export viewport, with reasonable minimum and maximum bounds.
Mobile exports continue to use the fixed 960px layout and 2x output for predictable high-resolution images.
Export-only normalization disables non-zero letter spacing because
html2canvas-prootherwise renders mixed CJK and Latin graphemes separately with different baselines.Inline-code text is wrapped in the cloned export DOM so its glyph alignment can be adjusted without moving the original border or background. Its export-only line height is normalized to prevent both user and assistant inline-code pills from inheriting an oversized chat line box.
Multi-image previews now use a non-wrapping flex layout with proportional shrinking, preserving the natural aspect ratios of landscape and portrait attachments.
Testing
bun typecheckbun run test:webAI disclosure
Implementation and testing were assisted by OpenAI GPT-5.6 Codex.