Skip to content

Shrink description box instead of image#31

Merged
elhil merged 6 commits into
mainfrom
fix-description-height
Feb 23, 2026
Merged

Shrink description box instead of image#31
elhil merged 6 commits into
mainfrom
fix-description-height

Conversation

@elhil
Copy link
Copy Markdown
Contributor

@elhil elhil commented Feb 6, 2026

I think I found an elegant solution to #28 using built-in properties of flex boxes. Using flex-shrink we can cause the description box to shrink down (to a minimum limit) based on the available space in the column, and for the image to not shrink at all.

This should allow the description box not to be truncated for short images, but will privilege the image when assigning space. It's possible I'm misunderstanding flex-shrink, and that there will still be some mismatches, at which point we can just set a fixed height for the description box. But I suspect this behaves exactly as we want it to.

Resolves #28

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 6, 2026

Test QC report built! You can download it here.

@elhil elhil requested a review from joshuacwnewton February 9, 2026 17:23
Comment thread src/ImageDisplay.tsx
@github-actions
Copy link
Copy Markdown

Test QC report built! You can download it here.

Copy link
Copy Markdown
Member

@joshuacwnewton joshuacwnewton left a comment

Choose a reason for hiding this comment

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

This is perfect! Approving as I think this is good to merge. :)

I tried some other bits of styling (e.g. having the infobox be a fixed size (i.e. flex-1) to lessen the jitteryness of the resizing infobox) but the empty space felt unnatural, and I like your approach the most.

As an aside, I did have some very minor nitpicks about spacing? But, these are super nitpicky/perfectionistic, and aren't really relevant for the scope of this PR, so we could always save them for a follow-up.

Details
  • When the text box is full-size, it doesn't line up with the buttons due to button spacing (blue space below buttons):

    Image
  • The text inside of the textbox looks the slightest bit off due to the text having spacing above the letters (blue space above "SCT"):

    Image
  • The spacing between elements and the outer edge of the viewport is not quite even on the left-right-bottom sides?

    Image Image

@github-actions
Copy link
Copy Markdown

Test QC report built! You can download it here.

@elhil
Copy link
Copy Markdown
Contributor Author

elhil commented Feb 13, 2026

Nits implemented! With the exception of this one:

  • The text inside of the textbox looks the slightest bit off due to the text having spacing above the letters (blue space above "SCT")

because this is a normal typographical thing related to line heights that I think has its own reasons for being there. We could do some hacky thing to line it up but I personally think it's fine as it is, especially now that the other stuff all lines up well.

Another thing I tried as an experiment is to make the description box a fixed height so it doesn't jitter when scrolling. Let me know how you feel about it: do we hate the empty space more than we love the lack of movement? That change is in its own commit so trivial to revert :)

@joshuacwnewton
Copy link
Copy Markdown
Member

joshuacwnewton commented Feb 13, 2026

Ah! I think there might be another viewport-related issue with the button spacing. When the window is made smaller, the buttons line up:

image

However when I pop out the dev console and enlarge the window to take up the entire 1080p width, I see:

image

And highlighting the outer div shows:

image

I appreciate you taking care of these nits. ♥️

@joshuacwnewton

This comment was marked as resolved.

@joshuacwnewton joshuacwnewton self-requested a review February 20, 2026 16:36
joshuacwnewton

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown

Test QC report built! You can download it here.

@joshuacwnewton joshuacwnewton dismissed their stale review February 20, 2026 16:38

I'm silly, and forgot that there was still an outstanding comment.

@github-actions
Copy link
Copy Markdown

Test QC report built! You can download it here.

@elhil
Copy link
Copy Markdown
Contributor Author

elhil commented Feb 20, 2026

Ah! I think there might be another viewport-related issue with the button spacing. When the window is made smaller, the buttons line up:

I hadn't noticed that since my local report was showing Command + Args and therefore the table was taking up more vertical space. Fixed by making the table take up all available space.

Copy link
Copy Markdown
Member

@joshuacwnewton joshuacwnewton left a comment

Choose a reason for hiding this comment

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

LGTM!

@elhil elhil merged commit a166344 into main Feb 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image size scales based on number of lines in the text description box

2 participants