Skip to content

Conversation

@alanpoon
Copy link
Contributor

@alanpoon alanpoon commented Jan 1, 2026

Fixes #646

  1. After loading the full-size image, it should fill the entire view by default.
  2. Remove max zoom.
  3. Make the background partially transparent instead of just opaque white.
    4.Make the rotation animation faster — it should take no more than 200-300 milliseconds, perhaps even faster.
  4. Reformat the buttons to look better. line up horizontally with the image metadata view on the upper left.
    We should use the RobrixIconButton widget such that the button style is consistent with other buttons. For example, look at the X close button on the settings screen.
  5. Add a "reset view" button that resets the pan/zoom settings to the default, which is just the image filling the view with no rotation.
    Handle the regular set of actions/gestures that close the modal: pressing Escape, the back navigational gesture on Android, etc.
  6. It seems like ModalAction::Dismissed is not emitted when Esc is pressed, causing unable to close the Modal. It could be due to Fill width and height for modal content. Currently, only this code event.back_pressed() || matches!(event, Event::KeyDown(KeyEvent { key_code: KeyCode::Escape, .. }) to ensure android back button and Keyboard Escape to close the modal.

@alanpoon alanpoon self-assigned this Jan 1, 2026
@alanpoon alanpoon added the enhancement New feature or request label Jan 1, 2026
@alanpoon alanpoon force-pushed the image_viewer_enhancement#646 branch from 4187cab to 153458f Compare January 1, 2026 08:39
@alanpoon alanpoon added waiting-on-review This issue is waiting to be reviewed and removed enhancement New feature or request labels Jan 2, 2026
@kevinaboos kevinaboos changed the title Image viewer enhancement#646 Image viewer enhancements Jan 3, 2026
@kevinaboos
Copy link
Member

Awesome, thanks! Will review soon.

  1. It seems like ModalAction::Dismissed is not emitted when Esc is pressed, causing unable to close the Modal. It could be due to Fill width and height for modal content. Currently, only this code event.back_pressed() || matches!(event, Event::KeyDown(KeyEvent { key_code: KeyCode::Escape, .. }) to ensure android back button and Keyboard Escape to close the modal.

Huh, that's weird. Might be a bug with modals.... which will be redesigned soon.

In general, we don't want to match on raw Event variants because it will match any event even if that widget doesn't have key focus; that's why we typically match on event.hits() instead.
However, in this case, this is probably ok because the ImageViewer is a full-screen widget that is always shown in front of others.

Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

The improvements look pretty good, thanks! Here's a screenshot of what I see, so I will make suggestions based on that:
image

Just a few more tweaks needed to make this sleeker:

  1. The buttons are all mashed together; please add some spacing between them.
    • Ensure that they are large enough and spaced nicely to be tapped accurately on a mobile device.
  2. For better contrast, we should use a transparent dark gray background. Use the same color as the background of the message context menu.
    • You could even make it darker (more opaque) than that if you want. We want contrast between the image and the background.
  3. In addition, please add a partially-transparent white background for the black metadata text, which will make the text readable atop the dark gray or image background.
    • Here's an example image showing why we need a background behind the text to make it more legible: image
  4. When first opening the ImageViewer, it doesn't properly display the image as Filling the entire view. As you can see in the above screenshot, it's far bigger than the view.
    • However, it doest work as expected when you click the Reset button. Thus, when opening the ImageViewer, you probably just need to automatically apply the same configuration that happens when the reset button is clicked.
  5. The metadata text is still being line-wrapped unnecessarily. See "Ferris Rustlang.png (94.1 KB)" in the top-most screenshot; there seems to be plenty of horizontal space for it to not be wrapped.
  6. This is something I forgot to mention in #646 (sorry about that), but we need to allow the user to temporarily hide the buttons and metadata text/avatar, such that they can view the image without the other content blocking it.
    • I'm sure you know how this works with most apps, but I'll describe the desired behavior below just in case.
      • On Mobile, tapping anywhere on the image will toggle whether the buttons/metadata are shown or hidden. This would ideally be done with a smooth fade to visible/invisible.
      • On Desktop it works the same way: clicking anywhere on the image will toggle whether the metadata/buttons were shown. In addition to that, if the mouse is moved, the buttons/metadata should be shown again for ~2 seconds.
      • After no activity for ~2 seconds (no taps, clicks, or mouse movement), the buttons and text should fade away and be auto-hidden.

In general, we need to remember to design this widget (and all others) to work well on mobile devices without an accurate mouse pointer, not just a desktop.

@kevinaboos kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Jan 3, 2026
@alanpoon alanpoon force-pushed the image_viewer_enhancement#646 branch from 6e89533 to 84c1496 Compare January 5, 2026 08:06
@alanpoon alanpoon force-pushed the image_viewer_enhancement#646 branch from 75bad6d to 853b6ed Compare January 6, 2026 08:47
@alanpoon
Copy link
Contributor Author

alanpoon commented Jan 6, 2026

Fix cluttering on mobile devices.
Screenshot 2026-01-06 at 5 13 13 PM

  1. Fixed cluttering
  2. Fixed background color
  3. Added partially-transparent white background
  4. Fixed image not displaying in Fill width and height with capped_width and capped_height
  5. Increase width of metadata text to a larger Fixed width.
  6. Added auto hiding of UI.

@alanpoon alanpoon added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Jan 6, 2026
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks, it generally looks a lot better now, but it's definitely still cluttered and a bit difficult to use.

Here's a screenshot, I will highlight some key things that need to change based on this.
image

  1. The overlay with metadata & buttons is too big and covers too much of the image. It just doesn't look very sleek or professional.
    • I think the best solution for this is to split the single view into two views: the button view at the top right, and the metadata view centered at the bottom. That way, it will be a lot easier to handle different screen sizes, and it will also look cleaner.
    • Both views should be RoundedViews with border_radius: 4.0, just like all other views in Robrix, and they should have a 10-20 pixel margin on all sides to give them a "floating" look (you could also optionally add shadows, but I don't think it's needed here). The current style is inconsistent with the rest of Robrix.
    • The auto-hide behavior (show on-hover, hide automatically or upon a tap) should still treat both views as one, in that you will show or hide them both together.
  2. We need even more contrast. Make the image background darker/more opaque, and make the white background even whiter and more opaque.
  3. The Avatar in the metadata view should not be clickable. You can prevent this by passing in None for the info parameter in Avatar::show_text()/show_image().
  4. The overlay UI currently auto-disappears even when the mouse is hovering over the buttons or metadata view. This shouldn't happen — if the mouse is over either the metadata view or the buttons view, both views should be shown. The 2-second timer for auto-hiding those views should start only once the cursor stops moving and is not hovering over those views.
  5. Changing the app window size breaks the image zoom feature in several different ways. Make sure you're updating the bounds/limits for the zoom and size calculations whenever the app window geom changes.
    • For example, the minimum size of the image (when it is furthest zoomed out) actually changes based on the size of the app window. So when the app window is smaller, I'm able to zoom out further than when the app window is larger. This doesn't really make sense, and shouldn't be the case.
    • When the image viewer is in the default/reset view mode (filling the screen), if I then change the app window size, the image doesn't automatically resize to fill the screen. If the ImageViewer is in its default state (we haven't zoomed or panned yet), then it should Fill the entire screen even when the window geom changes.

@kevinaboos kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-author This issue is waiting on the original author for a response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve the full-screen ImageViewer

2 participants