Skip to content
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

Enhancement: Hide photo viewer overlay after zooming an image #16303

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

aleksusklim
Copy link

Fixes #7931.

In OverlayWidget, I've added new ControlsState called ControlsDisabled, which locks visibility of all overlay controls and prevents mouse interaction for anything other than previous/next navigation and close button.

It is enabled when user zooms in the image or presses reset-zoom button (middle mouse). Then all overlay controls are instantly disappearing, while mouse cursor will be permanently visible (to allow the user to click on navigation buttons, though invisible).

After zooming out the image or after switching to another photo, the overlay controls will appear gradually.

To make this feature more accessible, I've also added extra hotkeys to change zoom level: NumPad Plus and NumPad Minus to zoom in and out (which currently required to hold Ctrl key) and NumPad Multiply to reset zoom level, just as middle mouse button (currently this is only possible by Ctrl+Zero, which is not working reliably for some reason).

Also for simple photos (not videos and not documents) the Enter and Space keys will reset zoom level, effectively enlarging and hiding controls. This is convenient, because after switching to another photo (left/right keys) the controls will always appear again.

@CLAassistant
Copy link

CLAassistant commented May 10, 2021

CLA assistant check
All committers have signed the CLA.

@@ -498,6 +499,7 @@ private Q_SLOTS:
ControlsShown,
ControlsHiding,
ControlsHidden,
ControlsDisabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ControlsDisabled
ControlsDisabled,

@23rd
Copy link
Collaborator

23rd commented May 10, 2021

Then all overlay controls are instantly disappearing, while mouse cursor will be permanently visible (to allow the user to click on navigation buttons, though invisible).

Such behavior is very confusing to the user. I'm afraid this is unacceptable.

2021-05-10.mp4

@aleksusklim
Copy link
Author

@23rd, It is not confusing as the user is knowing what he is doing. The mouse cursor exactly shows that the "button" is under.

Often when a bright image is zoomed in – you can't clearly see those arrows anyway!

Hm-m, do you think that the button should appear when hovering over it? I'm not sure it is easy to make as is, but it's certainly possible in theory,

@23rd
Copy link
Collaborator

23rd commented May 10, 2021

It is not confusing as the user is knowing what he is doing. The mouse cursor exactly shows that the "button" is under.

This is just my opinion. I think only the bottom info should be hidden, the navigation buttons and the cross button should keep their behavior unchanged.

It is enabled when user zooms in the image or presses reset-zoom button (middle mouse).

Perhaps the bottom info should only be hidden when it overlaps content.

@aleksusklim
Copy link
Author

But that's the point of «fullscreen view» – to view something is a full screen, without obscuring it with interface elements. (Also note that this media viewer currently doesn't allow user to drag something out of screen borders: for example, I cannot move zoomed-in picture down-left to make its top-right corner NOT snapped to screen).

Even if the user becomes confused, a single click into the picture will be enough to close the viewer instantly!

Wait, I have an idea: scratch this approach and make it differently: let the «disappear timeout for controls» much shorter when the image was zoomed. I will test that to see how it fits…

@23rd
Copy link
Collaborator

23rd commented May 10, 2021

But that's the point of «fullscreen view» – to view something is a full screen, without obscuring it with interface elements.

Well, then maybe adding the fullscreen mode for photos would be enough for you? Add the same Ctrl + F shortcut, then advanced users will "know what they are doing".

@aleksusklim
Copy link
Author

@23rd, I've made a second attempt. Now I don't block the controls, just shorten their «hide time» if the image was zoomed in.

Also, I prevent controls from disappearing if one of them is under the mouse cursor.

Finally, I've removed all proposed keyboard binding, instead enabling Plus, Minus and Zero keys to zoom simple photos without held Ctrl key.

Co-authored-by: 23rd <23rd@vivaldi.net>
@23rd
Copy link
Collaborator

23rd commented May 13, 2021

I'm afraid this spoils the user experience even more.

2021-05-13.mp4

I see two options here.

  • A hard way: leave navigation + cross buttons and hide the bottom information only when it overlaps the content.

  • An easy way: add a fullscreen mode (Ctrl + F) for photos in the same way as for video. In this mode, for advanced users, the whole interface would be hidden.

But remember, the final decision is always up to Preston. I'm just giving my opinion.

@aleksusklim
Copy link
Author

And what's the problem with the mouse cursor? Visible when moving, hidden right after that.

Welp, we can make it visible the whole time while isZoomedIn() returns true. Would that suffice?

One thing I don't like is that right-down menu will still pop-up, preventing the user to put the mouse there. Since this is the same menu that brings up after right click – maybe remove that button from the screen in zoomed state?

This way the user could put the cursor to right-down corner effectively hiding it, and not overlapping with any controls. The menu is still accessible with right click.

I'll try to add this behavior to my commits...

@23rd
Copy link
Collaborator

23rd commented May 14, 2021

Please consider your changes for all possible displayed content, including documents and themes.

@aleksusklim
Copy link
Author

I tested my code with albums, videos, gifs and files.

For albums: previews of other images in the album (just as the post caption text) are disappearing with all other controls, but they do not prevent interface hiding even when the mouse cursor is over them. I think it is acceptable, because they can fill a large part of the screen, so it is good that they are transparent for hover detection.

For videos, the main playback controls are not disappearing in zoomed state. Also the ResetZoom button sometimes does not enlarge the video to full screen. But the video has its own fullscreen mode, where new behavior has no effect (so the controls will hide after a default delay). Zooming video not in full screen still triggers quicker hiding of anything other than playback controls.

For GIFs, the behavior is identical to photos, except that new simplified key-bindings for zoom are requiring holding the Ctrl key. Other than that, a zoomed-in GIF can be dragged and viewed just as photo (hiding controls), but also can be paused by Space (current TDesktop behavior unchanged).

For files (documents that failed to display as media content, for example videos with unknown codec, some large WEBP and several PDF files) the zoom does not making sense, because it will do nothing: TDesktop will just show a preview (or generic icon) with SaveAs link. Anyway, after a forced zoom-in event, the other controls will hide quicker (because the proposed code does not handle this case separately), but I believe this brings no harm to user experience.

Some invalid media may be displayed as 1x1 transparent pixel, which can be zoomed/maximized, hiding overlay controls. But again, this is the expected behavior.

Thus I think the proposed code is production-ready. The only tweak that could be done further – is the adjustment of new quicker delays. If you think that the delays are better be different – you can change them as you like. For now, I left them all equal to 150 ms.

@23rd
Copy link
Collaborator

23rd commented May 14, 2021

For files (documents that failed to display as media content, for example videos with unknown codec, some large WEBP and several PDF files) the zoom does not making sense, because it will do nothing: TDesktop will just show a preview (or generic icon) with SaveAs link. Anyway, after a forced zoom-in event, the other controls will hide quicker (because the proposed code does not handle this case separately), but I believe this brings no harm to user experience.

If zooming in doesn't change anything for files, then it shouldn't speed up controls hiding, should it?

@aleksusklim
Copy link
Author

If zooming in doesn't change anything for files, then it shouldn't speed up controls hiding, should it?

It does not making any difference. The user has no reason to zoom, and even if he'll do it – nothing breaks. What's the problem?

Displaying of invalid documents – is an exceptional situation. Like, there is a button to «Rotate» a picture. It does nothing for documents. I could say, «Hey, then this button should be hidden if it is not working!»

But it's such a minor detail that hurts noone.

@23rd
Copy link
Collaborator

23rd commented May 14, 2021

Displaying of invalid documents – is an exceptional situation.

But displaying themes — is not.

@aleksusklim
Copy link
Author

How to test a "theme" on test cloud?

@23rd
Copy link
Collaborator

23rd commented May 14, 2021

How to test a "theme" on test cloud?

You can upload a file of theme to Saved Messages.

@aleksusklim
Copy link
Author

aleksusklim commented May 14, 2021

OK, I've tested it. (I have noticed that pop-up menu in right-down corner is overlapped by «APPLY THIS THEME» but it is irrelevant here).

The theme preview does not zooming either. So, «zoom does nothing». Oh, wait! It shortens the delay of hiding overlay controls.

So, when the users become aware that middle mouse click effectively «goes fullscreen» and hides the overlay – they will use this behavior for themes preview too.

For example, now I saw the whole theme preview thing for the first time – and the overlay still obscured some of visible elements. Especially when the interface scale is increased like mine. Yet, the bottom panel (cancel/apply) at large scale is interfering way too severely! Example at 125% on 1024*768 screen, theme MossAmbrosia:

MossAmbrosia_1.png

MossAmbrosia_1.png

Though for default size it looks pretty neat:

MossAmbrosia_2.png

MossAmbrosia_2.png

And I don't feel anything wrong with triggering quicker hiding of controls (at least some of them) after a middle-click:

MossAmbrosia_3.png

MossAmbrosia_3.png

This is how the bug with an overlapped menu looks like, in different scales:

MossAmbrosia_4.png

MossAmbrosia_4.png

I mean, if you want to have less small visual glitches, then it's good to get rid of them. But it should not delay the implementation of new independent features and enhancements.

@lokpozzta
Copy link

m: Telegram Support login+canned.response@stel.com
Date: 20 June 2021 at 3:04:40 AM GMT+7
To: sreyleak2703@icloud.com
Subject: Re: Invalid phone number: +855 1122562

@Aokromes
Copy link
Collaborator

Aokromes commented Jun 19, 2021

m: Telegram Support login+canned.response@stel.com
Date: 20 June 2021 at 3:04:40 AM GMT+7

you leaked your phone number.

Copy link

@Sirflyzoner Sirflyzoner left a comment

Choose a reason for hiding this comment

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

  • thank u

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.

Enhancement: Hide photo viewer overlay after zooming an image
7 participants