GNOME Bugzilla – Bug 700574
display-page: Show overlay toolbar until mouse moves
Last modified: 2016-03-31 13:22:07 UTC
See patch. About the FIXME in the patch, I spent about an hour trying to figure it out but failed. :( However, I don't think its really a big deal but anyone is welcome to help me figure out a way to avoid the idle call.
Created attachment 244614 [details] [review] display-page: Show overlay toolbar until mouse moves Based on the user feedback, it seems that its very easy to miss the fact that there is a toolbar available in fullscreen mode, especially now that we go to fullscreen automatically. Better keep the overlay toolbar visible until user moves the mouse to ensure that they at least know it exists and where.
I haven't been able to run meaningful tests of this as jhbuild master/f19 3.8 behaviour differed too much. On master, some toolbar tend to be shown most of the time after switching to fullscreen, but it does not look like the fullscreen toolbar. Assuming the issue is that when going to fullscreen, we don't show the toolbar until the user moves the mouse to the top of the screen, then I'm not sure this patch is a great way to improve that. I know that right after creating a VM, I keep moving the mouse until I realize the VM wants keyboard input. This means for me the popup would disappear very quickly. I think it would be better to show it for N seconds, and then have it disappear. Another random thought: maybe we should always show the toolbar when we have no agent running as in these cases the resolution is very unlikely to be more than 640x480 (?)
(In reply to comment #2) > I haven't been able to run meaningful tests of this as jhbuild master/f19 3.8 > behaviour differed too much. On master, some toolbar tend to be shown most of > the time after switching to fullscreen, but it does not look like the > fullscreen toolbar. I forgot that something weird happens when mouse is server-side: we show the non-fullscreen toolbar even in fullscreen. I'll investigate.. > Assuming the issue is that when going to fullscreen, we don't show the toolbar > until the user moves the mouse to the top of the screen, then I'm not sure this > patch is a great way to improve that. I know that right after creating a VM, I > keep moving the mouse until I realize the VM wants keyboard input. This means > for me the popup would disappear very quickly. I think it would be better to > show it for N seconds, and then have it disappear. Actually my first thought was exactly that but then I thought user could easily miss it. So I think I'll mix these ideas and make it disappear after a few seconds of mouse move. > Another random thought: maybe we should always show the toolbar when we have no > agent running as in these cases the resolution is very unlikely to be more than > 640x480 (?) Well it could also be that agent is just not installed (think windows vista and 8).
Created attachment 245006 [details] [review] display-page: Don't make overlay toobar invisible unconditionally It seems we were making overlay toolbar unconditionally invisible each time visibility of docked (is that the correct word) toolbar was changed. Since docked toolbar in always visible when not in fullscreen and invisible in windowed-mode, deciding its visibility based on mouse grab doesn't make any sense and code doing that was not functional. With this patch, we decide the visibility of the overlayed toolbar on mouse grab, instead.
(In reply to comment #3) > (In reply to comment #2) > > I haven't been able to run meaningful tests of this as jhbuild master/f19 3.8 > > behaviour differed too much. On master, some toolbar tend to be shown most of > > the time after switching to fullscreen, but it does not look like the > > fullscreen toolbar. > > I forgot that something weird happens when mouse is server-side: we show the > non-fullscreen toolbar even in fullscreen. I'll investigate.. With the attached patch, this is now fixed. > > Assuming the issue is that when going to fullscreen, we don't show the toolbar > > until the user moves the mouse to the top of the screen, then I'm not sure this > > patch is a great way to improve that. I know that right after creating a VM, I > > keep moving the mouse until I realize the VM wants keyboard input. This means > > for me the popup would disappear very quickly. I think it would be better to > > show it for N seconds, and then have it disappear. > > Actually my first thought was exactly that but then I thought user could easily > miss it. So I think I'll mix these ideas and make it disappear after a few > seconds of mouse move. Actually it turns out that the (existing) code that makes the toobar invisible on mouse move already does it with a timeout. This timeout is the usual app-wide animation duration, App.app.duration. This is a good timeout value for the case of toolbar hiding when taking mouse away from it, its not good for this case. I'll fix this now..
Created attachment 245007 [details] [review] display-page: Show overlay toolbar until mouse moves Based on the user feedback, it seems that its very easy to miss the fact that there is a toolbar available in fullscreen mode, especially now that we go to fullscreen automatically. Better keep the overlay toolbar visible until after 1 second of user moving the mouse to ensure that they at least know it exists and where.
I still can hit some cases where the fullscreen toolbar is not shown initially when connecting to a freebsd 6.0 installer stuct on the very first text screen shown at boot from the collection view, I'm not exactly sure what's happening.
Review of attachment 245006 [details] [review]: s/Since docked toolbar in always visible/Since docked toolbar is always visible/ in log Fwiw, while testing I've seen cases where the mouse could be/was grabbed was the VM, and the overlaid toolbar was not visible even though I was in fullscreen (seemed to happen with unknown ISOs, eg freebsd 6.0 installer). Looks good otherwise, is Marc-André fine with it?
Review of attachment 245007 [details] [review]: ::: src/display-page.vala @@ +314,3 @@ + Idle.add (() => { + update_toolbar_visible (); + overlay_toolbar_invisible_timeout = 1000; // 1 seconds I'd make that longer as if you keep moving the mouse after having clicked on 'create' or on the box you want to connect to, the toolbar will be gone by the time boxes has connected to the VM.
Review of attachment 245006 [details] [review]: the toolbar should be visible if the mouse can be grabbed, no overlay. Apparently this patch is changing this. I know this used to work, but it might have been broken, but it looks like it's not going where it should.
Review of attachment 245006 [details] [review]: Why would we show non-overlay toolbar in fullscreen?
Review of attachment 245006 [details] [review]: > Why would we show non-overlay toolbar in fullscreen? because the overlay toolbar is not reachable if the mouse is grabbed by the display widget. Using non-overlay toolbar also makes it more clear that you are in some "minor/diminished" mode.
Review of attachment 245006 [details] [review]: >> Why would we show non-overlay toolbar in fullscreen? >because the overlay toolbar is not reachable if the mouse is grabbed by the display widget. Using non-overlay toolbar also makes it more clear that you are in some "minor/diminished" mode. That assumes that overlay is only displayed on mouse hover while this patch makes it so that it is always visible when mouse can be grabbed rather than nonoverlay toolbar. I don't see showing of non-overlay as any indication of "minor/diminised" mode, it just looks weird to show that in fullscreen and inconsistent IMO. BTW, showing of (either) tooblar when mouse can be grabbed is broken (w/ and w/o this patch) for me at least. I don't even know why its on 'can-grab' rather than 'grabbed'. The following works and is better IMO: @@ -247,3 +247,3 @@ private void update_toolbar_visible() { toolbar.visible = false; - set_overlay_toolbar_visible (can_grab_mouse); + set_overlay_toolbar_visible (grabbed); } else { @@ -308,2 +308,3 @@ public void show_display (Boxes.Display display, Widget widget) { update_title (); + update_toolbar_visible (); return false;
Created attachment 245044 [details] [review] display-page: Show toolbar when mouse is grabbed Rather than when it can be grabbed. This not only works compared to the previous approach but it also better as there is no need to always show toolbar just because mouse can be grabbed.
If you show the (nonoverlay) toolbar only when the mouse is grabbed, you are making the toolbar dyanmic, appear and disappear. This is annoying. The UI should be fixed.
It's rather simple: If mouse can be grabbed: no overlay, just regular toolbar. It never get hidden, you always have binding clearly visible. The toolbar is not reachable if the mouse is grabbed. If mouse is client side, you can use fancy overlay, no need for binding info. The overlay is always reachable.
(In reply to comment #15) > If you show the (nonoverlay) toolbar only when the mouse is grabbed, you are > making the toolbar dyanmic, appear and disappear. This is annoying. Thats a good point. > The UI > should be fixed. From what i can tell, somehow SpiceDisplay.can_grab_mouse returns 'false' even when mouse is grabbable. I'll investigate more..
(In reply to comment #9) > Review of attachment 245007 [details] [review]: > > ::: src/display-page.vala > @@ +314,3 @@ > + Idle.add (() => { > + update_toolbar_visible (); > + overlay_toolbar_invisible_timeout = 1000; // 1 seconds > > I'd make that longer * I tried diff values. In some guests overlay hides important controls on the top (e.g gnome3) and its them being most accessed UI components in the guest, user is very likely to want to click on them soon after connecting to the VM. This includes use case of connecting to the VM just to cleanly restart/shutdown the VM. > as if you keep moving the mouse after having clicked on > 'create' or on the box you want to connect to, the toolbar will be gone by the > time boxes has connected to the VM. We are setting this up during the actual display connection phase so this doesn't happen (probably cause connecting to display is fast).
Created attachment 245073 [details] [review] display-page: Update toolbar visibility on showing the display The code to make toolbar visible on mouse becoming grabable wasn't working when mouse was grabable before display was shown. This patch fixes that by updating the toolbar visibility each time display is shown.
(In reply to comment #18) > > > as if you keep moving the mouse after having clicked on > > 'create' or on the box you want to connect to, the toolbar will be gone by the > > time boxes has connected to the VM. > > We are setting this up during the actual display connection phase so this > doesn't happen (probably cause connecting to display is fast). I've seen it with the first iteration of the patches in this bug, so this is happening (though the docked toolbar is being shown at first for a few seconds, and is then replaced by the fullscreen toolbar). Display connection is not very fast here (a few seconds between the first part of the screen zoom and the actual connection).
(In reply to comment #20) > (In reply to comment #18) > > > > > as if you keep moving the mouse after having clicked on > > > 'create' or on the box you want to connect to, the toolbar will be gone by the > > > time boxes has connected to the VM. > > > > We are setting this up during the actual display connection phase so this > > doesn't happen (probably cause connecting to display is fast). > > I've seen it with the first iteration of the patches in this bug, so this is > happening (though the docked toolbar is being shown at first for a few seconds, > and is then replaced by the fullscreen toolbar). Display connection is not very > fast here (a few seconds between the first part of the screen zoom and the > actual connection). 1. The first iteration of the patches didn't use a 1 second timeout but rather a few milliseconds. 2. We are showing the toolbar and setting up the timeout at DisplayPage.show_display() which is called *after* the connecting phase. So I don't see how you can manage to make the toolbar invisible before you can see it by moving the mouse very early.
Review of attachment 245007 [details] [review]: I agree timeout could be longer, 4-5 seconds wouldn't be so bad.otherwise, works like advertized, ack
Review of attachment 245073 [details] [review]: ack
(In reply to comment #22) > Review of attachment 245007 [details] [review]: > > I agree timeout could be longer, 4-5 seconds wouldn't be so bad. I explained the problem I see with bigger timeouts in comment#18. > otherwise, > works like advertized, ack Thanks for trying it out.
Attachment 245007 [details] pushed as 1edaf23 - display-page: Show overlay toolbar until mouse moves We can increase the timeout later if needed. Attachment 245073 [details] pushed as 48640be - display-page: Update toolbar visibility on showing the display