-
Notifications
You must be signed in to change notification settings - Fork 41
Image viewer enhancements #648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Image viewer enhancements #648
Conversation
4187cab to
153458f
Compare
|
Awesome, thanks! Will review soon.
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 |
There was a problem hiding this 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:

Just a few more tweaks needed to make this sleeker:
- 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.
- 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.
- 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.
- 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.
- 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.
- 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.
- I'm sure you know how this works with most apps, but I'll describe the desired behavior below just in case.
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.
6e89533 to
84c1496
Compare
75bad6d to
853b6ed
Compare
There was a problem hiding this 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.

- 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 withborder_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.
- We need even more contrast. Make the image background darker/more opaque, and make the white background even whiter and more opaque.
- The Avatar in the metadata view should not be clickable. You can prevent this by passing in
Nonefor theinfoparameter inAvatar::show_text()/show_image(). - 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.
- 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.


Fixes #646
4.Make the rotation animation faster — it should take no more than 200-300 milliseconds, perhaps even faster.
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.
Handle the regular set of actions/gestures that close the modal: pressing Escape, the back navigational gesture on Android, etc.
event.back_pressed() || matches!(event, Event::KeyDown(KeyEvent { key_code: KeyCode::Escape, .. })to ensure android back button and Keyboard Escape to close the modal.