-
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 |
kevinaboos
left a comment
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)", 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.

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.