Skip to content

Improve UX for the lightbox.#11783

Merged
vorporeal merged 5 commits into
masterfrom
david/improve-lightbox-ux
May 30, 2026
Merged

Improve UX for the lightbox.#11783
vorporeal merged 5 commits into
masterfrom
david/improve-lightbox-ux

Conversation

@vorporeal
Copy link
Copy Markdown
Contributor

@vorporeal vorporeal commented May 27, 2026

Description

This addresses some UX feedback from Erica:

  1. Clicking too close to the image doesn't trigger the background scrim dismiss behavior.
  2. When using a light theme, the buttons render oddly.

This fixes the feedback in the following ways:

  1. Add opt-in support to Image to utilize the image size when doing layout, to ensure the bounding box for the element is tight to the actual rendered image bounds.
  2. Define a custom button theme for lightbox buttons that is identical to the Secondary button theme when using the Dark application theme.

Fixes #11785.

Screenshot

image

Testing

  • Tested manually.

@cla-bot cla-bot Bot added the cla-signed label May 27, 2026
@vorporeal vorporeal requested a review from danielpeng2 May 27, 2026 23:01
Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vorporeal vorporeal marked this pull request as ready for review May 27, 2026 23:04
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 27, 2026

@vorporeal

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR tightens lightbox image layout to the rendered image bounds and introduces a lightbox-specific button theme so controls remain styled for the dark scrim regardless of the app theme. The attached screenshot satisfies the repository requirement for visual evidence on this user-facing change.

Concerns

  • No blocking correctness, security, or spec-alignment concerns found in the annotated diff.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

) -> Vector2F {
let size = constraint.max;
let mut size = constraint.max;
// If we want to lay out the image using the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment got cut off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh i thought we fixed this 😭

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh actually this might have been me getting distracted in the middle of writing a comment 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the old bug would happen when editing existing lines, which isn't the case here, so maybe this is a separate issue?

@vorporeal vorporeal force-pushed the david/improve-lightbox-ux branch from 0463db1 to ec85925 Compare May 30, 2026 00:48
@vorporeal vorporeal enabled auto-merge (squash) May 30, 2026 00:48
@vorporeal vorporeal merged commit 2e5ff6f into master May 30, 2026
27 checks passed
@vorporeal vorporeal deleted the david/improve-lightbox-ux branch May 30, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image lightbox Esc button and click-outside dismissal behave incorrectly

2 participants