feat(web): a lightbox for the feature screenshots, and the third font preloaded - #335
Merged
Conversation
… preloaded
TWO ITEMS FROM THE SITE REVIEW, plus a request.
THE LIGHTBOX. Every screenshot on /features is 2880x1800 rendered into roughly
half a column, so the UI detail the page is arguing about — a track matrix, a
loudness target — is unreadable at the size it is shown. Clicking now expands it
to fit the viewport, and Escape, the close button or the backdrop puts it back.
Built on <dialog> rather than a div, because the three things a hand-rolled
overlay reliably gets wrong come free: Escape closes it, focus is trapped while
open, and focus returns to the button that opened it. ::backdrop is a real
element so the dimming needs no extra node.
One shared dialog for all six shots rather than six dialogs. The buttons carry
the source and the caption; the dialog fills itself in. Its src is dropped on
close so a 2880x1800 decode is not held for something nobody is looking at.
Progressive: with JavaScript off the images render exactly as before. The button
is an affordance, and nothing depends on it. The trigger wraps the whole image
rather than sitting beside it — a small magnifier icon would be a worse target
than the 2880-wide picture it sits on.
Verified in a real browser, not by reading the markup: opens with the right
image and caption, focus lands on the close button, Escape closes AND returns
focus to the trigger it came from, the close button closes, a backdrop click
closes, and a click inside the box does not.
margin: auto is stated explicitly, and that is not decoration. Tailwind's
preflight resets the UA stylesheet's `dialog { margin: auto }` to 0, so a modal
dialog pins to the top left: measured at left=0 with a 167px gap on the right in
a 1200px viewport. With the line, 76px each side of the layout viewport.
THE FONT. jetbrains-mono was the one face not preloaded, and it is above the
fold on every page — the hero eyebrow, the `docker run` line a visitor copies,
the footer column labels. Measured on the live site over Slow 4G it began
loading 592ms after its two preloaded siblings, because the browser only learned
it was needed once the stylesheet had been parsed. No extra bytes; the file is
fetched either way.
There was a problem hiding this comment.
Pull request overview
This PR improves the /features page UX by adding a click-to-expand lightbox for the large feature screenshots, and optimizes font loading by preloading the JetBrains Mono font that’s used above-the-fold across the site.
Changes:
- Wrap feature screenshots in a trigger button and add a shared
<dialog>-based lightbox component to display the expanded image + caption. - Add a preload hint for
/fonts/jetbrains-mono.woff2in the base layout. - Add lightbox-specific styles and client-side behavior (open/close, backdrop click, and src cleanup on close).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/src/pages/features.astro | Wraps each screenshot in a lightbox trigger and mounts the shared Lightbox component once at the end of the page. |
| web/src/layouts/Base.astro | Preloads JetBrains Mono to reduce delayed font discovery above-the-fold. |
| web/src/components/Lightbox.astro | Introduces the shared <dialog> lightbox with styles and client-side open/close behavior. |
Suppressed comments (1)
web/src/components/Lightbox.astro:61
- The image sizing uses a hard-coded
calc(92vh - 3.25rem)budget for the caption. With the current caption typography/padding and the (often 2-line) alt text used as a caption, this can clip the caption or force overflow on smaller viewports. With.lightboxas a flex column, let the image take the remaining space instead of using a magic number.
.lightbox img {
display: block;
max-width: 100%;
max-height: calc(92vh - 3.25rem);
object-fit: contain;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+37
to
+53
| .lightbox { | ||
| /* max-width/height rather than width: a 2880x1800 shot must fit the viewport | ||
| on a laptop without cropping, and `contain` keeps the aspect ratio. */ | ||
| max-width: min(96vw, 1600px); | ||
| max-height: 92vh; | ||
| padding: 0; | ||
| /* EXPLICIT, because Tailwind's preflight resets the UA stylesheet's | ||
| `dialog { margin: auto }` to 0 and a modal dialog then pins to the top | ||
| left. Measured before this line: left=0, right gap=167px in a 1200px | ||
| viewport. */ | ||
| margin: auto; | ||
| border: 1px solid var(--color-line, #2a3140); | ||
| border-radius: 0.75rem; | ||
| background: var(--color-surface, #12151c); | ||
| color: var(--color-fg, #edf2f8); | ||
| overflow: hidden; | ||
| } |
The first version animated only the opening, with a keyframe on [open], and
left the backdrop to snap. A keyframe can only play forwards: close() removes
the dialog from the top layer immediately, so the picture vanished while the
dimming was still there. It read as a flash rather than a transition.
Rebuilt as a transition in both directions:
- `allow-discrete` on `display` and `overlay` keeps the element rendering for
the length of the exit, which is what makes a close animation possible at
all. Measured: `display` is still `block` on the frame after close(), and
opacity passes through 0.25 before reaching 0.
- `@starting-style` supplies the pre-open values, which are otherwise
unexpressible for an element that was not previously rendered.
- The backdrop is driven from the same [open] state, so the dimming and the
picture move together. Measured 0 -> 0.663 -> 0.8.
Travel as well as scale -- 6px of rise -- on a cubic-bezier that decelerates,
because a pure fade at this size reads as a swap rather than a movement.
Browsers without @starting-style skip to the end state, which is exactly the
behaviour before this commit: it appears and disappears. Nothing breaks.
Reduced motion keeps the dimming and drops the rest. The dimming is not
decoration -- it is what says the page behind is inert -- so it survives; the
scale and the travel do not. Verified the rules reach the built stylesheet with
transform: none and a 1ms transition.
|
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.
Two items from the site review, plus the lightbox request.
Lightbox on
/featuresEvery screenshot is 2880×1800 rendered into roughly half a column, so the UI detail the page is arguing about — a track matrix, a loudness target — is unreadable at the size it's shown. Clicking expands it to fit; Escape, the close button or the backdrop puts it back.
Built on
<dialog>, not a div, because the three things a hand-rolled overlay reliably gets wrong come free: Escape closes it, focus is trapped while open, and focus returns to the trigger on close.::backdropis a real element, so dimming needs no extra node.One shared dialog for all six shots — the buttons carry the source and caption. Its
srcis dropped on close so a 2880×1800 decode isn't held for something nobody is looking at.Progressive: with JS off the images render exactly as before. The trigger wraps the whole image rather than sitting beside it — a small magnifier icon would be a worse target than the picture it sits on.
Verified in a real browser, not by reading markup
One non-obvious line
margin: autois stated explicitly. Tailwind's preflight resets the UA stylesheet'sdialog { margin: auto }to0, so a modal dialog pins to the top left — measured atleft=0with a 167px right gap in a 1200px viewport. With the line: 76px each side.Font preload
jetbrains-monowas the one face not preloaded, and it's above the fold on every page — the hero eyebrow, thedocker runline a visitor copies, the footer labels. Measured on the live site over Slow 4G it began loading 592 ms after its two preloaded siblings. No extra bytes; the file is fetched either way.