docs(FR-2907): tighten screenshot capture guidance (parent-container + small-element rules)#7453
Closed
yomybaby wants to merge 4 commits into
Closed
docs(FR-2907): tighten screenshot capture guidance (parent-container + small-element rules)#7453yomybaby wants to merge 4 commits into
yomybaby wants to merge 4 commits into
Conversation
…apture guidance - markdown-processor-web.ts: emit max-width=(pixel_width × 0.5) from PNG header so 2× zoom captures display at intended CSS size. Mirrors the PDF auto-scale logic. Explicit  size hints still win. - styles-web.ts / styles.ts: move border/radius/shadow from .doc-image (img) to figure.doc-figure (wrapper); add padding + neutral background so the wrapper acts as a matte. Element-level captures with content flush to the PNG edges now have visible breathing room without any recapture. Cards/modals with their own rounded corners no longer compete with a second outer radius. - markdown-processor.ts: export getImageDimensions + IMAGE_SCALE_FACTOR so the web renderer can reuse the PDF dimension/scale helpers. - SCREENSHOT-GUIDELINES.md: document the matte, the renderer-side auto size cap, the parent-container-preferred rule, and the small-element rule (≤ 600 CSS px). - docs-screenshot-capturer.md: prompt update — capture raw elements and let the matte frame them, prefer parent containers, handle small widgets via auto-cap or browser_evaluate repositioning.
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Backend.AI documentation toolkit and screenshot authoring guidance to support matte-framed documentation images and automatic natural-size display caps for captured screenshots.
Changes:
- Adds matte-style image framing for web and PDF documentation output.
- Exports shared image dimension/scale helpers and applies automatic web image max-width caps.
- Updates screenshot capture guidance for modal framing, small widgets, and renderer-side sizing behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/backend.ai-webui-docs/SCREENSHOT-GUIDELINES.md |
Adds authoring guidance for matte framing, parent-container captures, and small-element sizing. |
packages/backend.ai-docs-toolkit/src/styles.ts |
Updates PDF image figure styling to use a matte frame. |
packages/backend.ai-docs-toolkit/src/styles-web.ts |
Updates web image figure styling to use a matte frame and bare inner images. |
packages/backend.ai-docs-toolkit/src/markdown-processor.ts |
Exports shared image dimension and image scale helpers. |
packages/backend.ai-docs-toolkit/src/markdown-processor-web.ts |
Adds web-side image dimension lookup and automatic max-width sizing. |
.claude/agents/docs-screenshot-capturer.md |
Updates screenshot capture agent guidance to align with the matte and sizing behavior. |
…er against traversal Copilot review on #7453: - styles-web.ts / styles.ts: `figure.doc-figure` was a default block, so the matte spanned the full article column even when the inner <img> was capped (e.g. a 760-px notification was rendered at 380 px inside a column-wide matte). Add `width: fit-content; max-width: 100%` so the matte shrinks to the largest child (capped image or caption), with `margin: ... auto` keeping it centered. - markdown-processor-web.ts: `resolveWebImageDiskPath` only checked that the href started with '/' before joining it under <srcDir>/<lang>. A '/../other-lang/foo.png' would slip past and let the renderer read PNG headers outside the language root just to compute display dimensions. `rewriteImagePathsForWeb` already filters out-of-tree refs, but enforce the same boundary at the resolver too (defense in depth).
Discovered while preparing a BEFORE/AFTER comparison on the live
sessions_all page. Two regressions hidden by the earlier fit-content
attempt:
1. Inline 'max-width: <cap>px' on the <img> won by specificity over
the .doc-image { max-width: 100% } default, so a 1335-px cap on a
552-px column let the image overflow the matte to the right.
2. figure { width: fit-content } sized the figure from the img's
MAX-CONTENT (intrinsic pixel width), ignoring any inline cap on
the img. Result: a 760-px notification with a 380-px cap still
sat inside a 616-px column-wide matte instead of a 412-px frame.
Both are fixed by moving the size cap from the img to the figure:
- markdown-processor-web.ts: emit
<figure style="max-width: min(<cap+32>px, 100%)">
The +32 reserves room for the 16-px matte padding × 2 so the
visible image width still matches the cap. The min(...,100%) keeps
the column boundary intact when the cap exceeds the column.
- markdown-processor.ts (PDF): same approach with +24 (12-px PDF
padding × 2).
- styles-web.ts / styles.ts: drop 'width: fit-content; max-width:100%'
from .doc-figure — the inline figure max-width drives sizing now,
and 'margin: ... auto' keeps the matte centered when narrower than
the column.
Verified on the rendered sessions_all page:
- session_type_batch: figure 552×410, img 518×344 (no overflow).
- launch_session_confirm: figure 616×742, img 582×675 (no overflow).
- session_notification: figure 412×162, img 378×95 (matte hugs the
capped image at column-independent size; column-wide pre-fix was
616×201).
yomybaby
added a commit
that referenced
this pull request
May 19, 2026
… guidance only User feedback after seeing the rendered output: the figure-level matte (off-white background + border + radius) plus the figcaption sitting inside the same frame ended up forcing odd page breaks in the PDF and read as awkward in both web and PDF. Reverting the renderer changes is cleaner than iterating on matte tuning. Scope this PR back to capture-time improvements only: - markdown-processor.ts / markdown-processor-web.ts / styles-web.ts / styles.ts: reverted to origin/main. No matte, no figure-side max-width cap, no auto-size for the web renderer. Renderer stays exactly as it was on main. - SCREENSHOT-GUIDELINES.md: rewrote the new 'Padding & Framing' section to reflect reality — the renderer does NOT add padding or auto-sizing, so any breathing room has to come from the capture itself. Kept the parent-container-preferred rule and the small-element rule, both now framed as 'capture a roomy parent OR reposition with browser_evaluate to bake padding into the PNG'. Added a short note about the existing inline size-hint syntax () as a last-resort override. - .claude/agents/docs-screenshot-capturer.md: same treatment. The 'matte handles it' assurances are gone; the agent is now instructed that the PNG it saves is what readers see, so any padding must be baked in via parent capture or browser_evaluate reposition. No changes to existing screenshots — adoption is incremental, applied when an image is next recaptured.
yomybaby
added a commit
that referenced
this pull request
May 19, 2026
yomybaby
changed the title
Member
Author
|
Abandoning this PR — the rendering-side fix turned awkward and the capture-side guidance alone didn't justify a standalone PR. Closing without merging. Existing screenshots stay as-is; future improvements will land via normal recaptures. |
yomybaby
added a commit
that referenced
this pull request
May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #7451(FR-2907)
Why
The original three problem captures on the Korean sessions_all page —
session_type_batch.png,launch_session_confirm.png,session_notification.png— share the same root cause: a Playwrightbrowser_take_screenshottaken with a tight elementref, producing a PNG whose content is flush to every edge.An earlier iteration of this PR tried to fix that at render time by wrapping every image in a "matte" frame (padding + soft background + outer border + radius) and auto-capping the display width. Visual review on the rendered docs site and the generated PDF made it clear that the matte ended up doing more harm than good — it forced odd page breaks in the PDF, double-bordered already-rounded captures on the web, and read as awkward overall. The cleaner fix is to capture better screenshots in the first place and leave the renderer alone.
What changes
Scope is now capture-time guidance only — zero renderer change.
packages/backend.ai-webui-docs/SCREENSHOT-GUIDELINES.mdNew "Padding & Framing" section that states up front:
Followed by two rules for capture authors:
Plus a short note about the existing `
` size-hint syntax as a per-image override.
The "Match the Existing Screenshot's Framing" table is updated to reference these rules.
.claude/agents/docs-screenshot-capturer.mdAgent prompt updated with the same model. The 'matte handles it' assurances are gone; the agent is now told explicitly: "The PNG you save is what the reader sees. Build any breathing room you want into the capture itself." The small-element rule lists path 1 (parent container) and path 2 (`browser_evaluate` reposition with a working snippet) — and is explicit that a bare ref screenshot of a sub-600-CSS-px element is not acceptable.
What was reverted
Renderer behaviour is byte-identical to main.
Adoption
Incremental. Existing screenshots are unchanged. When an image is next recaptured (e.g. as part of a separate i18n / UI-refresh pass) the new guidance applies. No bulk backfill in this PR.
Visual: how the new guide changes a capture
Same UI element (`.ant-card` containing the Session Type radio group on `/session/start`) captured two ways at 2× zoom:
The renderer wraps both PNGs identically (existing 1-px border, no padding). The visible difference is entirely the choice the capture author made.
Verification
Follow-up
FR-2908 ("intent-based alt text + figcaption separation") still planned, no longer blocked on renderer changes — independent now.
Checklist: