After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 663661 - Hot corner should have a little timeout
Hot corner should have a little timeout
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.2.x
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 638898 665436 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-11-09 00:14 UTC by Federico Mena Quintero
Modified: 2013-03-04 22:54 UTC
See Also:
GNOME target: 3.8
GNOME version: ---


Attachments
Add a timeout to hot corner's auto-activation (3.25 KB, patch)
2011-11-09 00:15 UTC, Federico Mena Quintero
reviewed Details | Review
Add a timeout to hot corner's auto-activation (3.29 KB, patch)
2011-11-11 17:54 UTC, Federico Mena Quintero
rejected Details | Review
Backport of patch to 3.0 (file affected and position of blocks changes, otherwise it's identical) (2.78 KB, patch)
2011-12-06 09:36 UTC, Dave Neary
rejected Details | Review
Hot corner analysis (135.52 KB, image/png)
2012-10-29 10:19 UTC, testify4
  Details
layout: Prevent going into negatives with the pressure (2.37 KB, patch)
2013-03-01 22:17 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Debounce triggering barriers (1.91 KB, patch)
2013-03-01 22:17 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Move the keybinding mode to the user of PressureBarrier (3.06 KB, patch)
2013-03-01 22:17 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Move tray-specific event filtration to the user of PressureBarrier (2.75 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Allow multiple barriers to contribute to a PressureBarrier (6.22 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Construct the primary monitors's hot corner, too (17.04 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Pass the X/Y coordinates when constructing the corner (2.18 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
layout: Put two barriers near every single hot corner (4.50 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
layout: Port the hot corner to pressure barriers (5.64 KB, patch)
2013-03-01 22:18 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
Activate overview with multiple monitor. (20.84 KB, image/png)
2013-03-03 12:44 UTC, Diogo Campos
  Details
layout: Extract toggling overview logic into one function (2.98 KB, patch)
2013-03-03 20:54 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
layout: Put two barriers near every single hot corner (4.50 KB, patch)
2013-03-03 20:54 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Don't use the corner's position for positioning ripples (994 bytes, patch)
2013-03-03 20:55 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Port the hot corner to pressure barriers (6.81 KB, patch)
2013-03-03 20:55 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
overview: Try to do the right thing related to XDnD (8.84 KB, patch)
2013-03-04 22:39 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
layout: Port the hot corner to pressure barriers (7.69 KB, patch)
2013-03-04 22:39 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
overview: Try to do the right thing related to XDnD (8.80 KB, patch)
2013-03-04 22:48 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Federico Mena Quintero 2011-11-09 00:14:36 UTC
I overshoot when flicking the mouse toward a File menu or the browser's Back button, often enough that the hot corner's instant activation becomes annoying.  This patch adds a little timeout to make the hot corner a little more forgiving.  For me 200 msec seems a bit short, and 300 a bit long - so I settled on 250 msec.
Comment 1 Federico Mena Quintero 2011-11-09 00:15:46 UTC
Created attachment 201038 [details] [review]
Add a timeout to hot corner's auto-activation

I constantly overshoot when flicking the mouse to a File menu or the browser's Back button.
This adds a little timeout to the hot corner, which lets me correct my overshoot as if
it were a normal mouse motion.  It doesn't seem to make the hot corner perceivably slower,
anyway, but see bgo#640186 for the bug about slowness while activating the overview.

Signed-off-by: Federico Mena Quintero <federico@gnome.org>
Comment 2 Rui Matos 2011-11-10 18:59:11 UTC
Review of attachment 201038 [details] [review]:

I'm on the fence here. While I think this is a good idea for people with sloppier hands/pointing devices it actually hinders the way I activate the overview which usually is a very quick mouse movement, i.e. the pointer is already on the window thumbnails area when the animation ends. This patch breaks that flow for me even with shorter timeouts.

As for the code, it looks good, but please remove the TABs.
Comment 3 Federico Mena Quintero 2011-11-11 17:54:19 UTC
Created attachment 201247 [details] [review]
Add a timeout to hot corner's auto-activation

I constantly overshoot when flicking the mouse to a File menu or the browser's Back button.
This adds a little timeout to the hot corner, which lets me correct my overshoot as if
it were a normal mouse motion.  It doesn't seem to make the hot corner perceivably slower,
anyway, but see bgo#640186 for the bug about slowness while activating the overview.

Signed-off-by: Federico Mena Quintero <federico@gnome.org>
Comment 4 Federico Mena Quintero 2011-11-11 17:57:14 UTC
(Sorry for the duplicated description; "git bz attach" simply put it there again.)

Tabs removed - I didn't notice them; my Emacs mode was screwed and the indent-tabs-mode declaration at the top of the file didn't catch on :)

So, should I push this?  I didn't think about mouse-ahead as in your situation.
Comment 5 drago01 2011-11-13 18:54:06 UTC
(In reply to comment #2)
> Review of attachment 201038 [details] [review]:
> 
> I'm on the fence here. While I think this is a good idea for people with
> sloppier hands/pointing devices it actually hinders the way I activate the
> overview which usually is a very quick mouse movement, i.e. the pointer is
> already on the window thumbnails area when the animation ends. 

Same here. The patch kind of breaks that.
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-11-13 19:02:39 UTC
Maybe we could detect the acceleration of the pointer and only have the timeout when the mouse is moving slow?
Comment 7 Federico Mena Quintero 2011-11-16 02:27:42 UTC
... or have a gsetting for this, not shown in the GUI?
Comment 8 Milan Bouchet-Valat 2011-12-03 13:00:29 UTC
(See also bug 665436, which is about preventing subsequent clicks on the Activities button by adding a timeout *after* the first click.)
Comment 9 Mathias Hasselmann (IRC: tbf) 2011-12-05 14:26:38 UTC
this discussion a bit reminds me of "oh, our new ui design doesn't work" - "ok, let's add more duct tape". maybe that hot corner isn't a good idea at all, if menu- and toolbars are placed in such way, that this hot corner becomes a problem? maybe the hot corner should be moved to a less conflicting place?
Comment 10 Dave Neary 2011-12-05 16:14:58 UTC
I also get a bit frustrated by this.

In the other corner, I occasionally (less often, but still) activate the notification area when I'm looking for the resize handle on windows that are maximized.

Mathais: I think the solution to creating less conflicting hot corners is not to remove the corner short-cuts, it's to figure out better ways for applications to do stuff like menus, "close windows" and resize handles, I think.

I'll try Federico's patch.

Dave.
Comment 11 Milan Bouchet-Valat 2011-12-05 16:32:22 UTC
(In reply to comment #9)
> this discussion a bit reminds me of "oh, our new ui design doesn't work" - "ok,
> let's add more duct tape". maybe that hot corner isn't a good idea at all, if
> menu- and toolbars are placed in such way, that this hot corner becomes a
> problem? maybe the hot corner should be moved to a less conflicting place?
While GNOME 3 has been much criticized, I heard surprisingly little complaints about the hot corner. So I don't think that's a really terrible design issue - it may just help some people.

Moving the hot corner elsewhere isn't really an option: that would mean changing the whole top panel, and the top-right corner is even worse because of the close button. As for the bottom-left corner, since there's no panel at that place, it would be hard to explain why a hot corner is there (and having a panel adds a few pixels to protect you from activating it when you don't want to).
Comment 12 Federico Mena Quintero 2011-12-05 20:50:52 UTC
(Note that gnome-panel used a timeout for auto-unhiding panels... since 2003 :)
Comment 13 Dave Neary 2011-12-06 09:35:11 UTC
Patch applied to panel.js in gnome-shell 3.0 - it definitely improves things, and small overshoots are forgiven, and I don't feel like I'm waiting for the corner when I want it. At 250ms, it still feels a little sensitive to me. But I agree with Federico, 300ms was a little too long.

I'll attach the patch for 3.0, for anyone who wants to apply it to an older version.
Comment 14 Dave Neary 2011-12-06 09:36:34 UTC
Created attachment 202903 [details] [review]
Backport of patch to 3.0 (file affected and position of blocks changes, otherwise it's identical)
Comment 15 Federico Mena Quintero 2011-12-09 15:06:15 UTC
See bug #665819 for the same thing in the opposite corner, for the notifications area.
Comment 16 Emmanuel Pacaud 2012-05-13 10:10:29 UTC
(In reply to comment #6)
> Maybe we could detect the acceleration of the pointer and only have the timeout
> when the mouse is moving slow?

One of the use case for this patch is with optical mice, sometimes the pointer jumps to the top left corner even with a small actual move of the device. So if the timeout is only active for small speed, it will be useless. It will also render the timeout useless for file menu selection overshoot.
Comment 17 Allan Day 2012-10-27 13:25:45 UTC
*** Bug 665436 has been marked as a duplicate of this bug. ***
Comment 18 Allan Day 2012-10-27 13:27:41 UTC
I think we should try and use pressure sensitivity for this, rather than a timeout. We should be able to do this after the next X11 release.
Comment 19 Federico Mena Quintero 2012-10-27 17:15:47 UTC
(In reply to comment #18)
> I think we should try and use pressure sensitivity for this, rather than a
> timeout. We should be able to do this after the next X11 release.

Do you have a URL (mailing list post or anything) about that feature?  I'm a bit disconnected from X11 happenings these days.
Comment 20 Jasper St. Pierre (not reading bugmail) 2012-10-27 17:18:50 UTC
(In reply to comment #19)
> Do you have a URL (mailing list post or anything) about that feature?  I'm a
> bit disconnected from X11 happenings these days.

I'm working on it. Check my GitHub page for the latest code dump.
Comment 21 Sri Ramkrishna 2012-10-29 05:03:59 UTC
Pressure sensitivity might help things since I don't rapidly mouse over.  The sensitivity will need to be messed around with to find the optimum settings..

When you have two monitors, and the main one is on the right not on the left this problem is worse.  I tend to arc my mouse movements when moving from one to the other.  It's the 'arc' that is the problem since it is a natural body movement.  Nobody moves over in a straight line.  If your menu strip is towards the top you'll see it right enough.

You could even plot this for predictability.  I do find it surprising that nobody complains.  By the way, I've been using an extension to change the sensitivity of the hotspot so I can play around with the numbers.  I think I have mine at 200ms or so.

Anyways, some additional insight to the problem.
Comment 22 testify4 2012-10-29 10:19:18 UTC
Created attachment 227510 [details]
Hot corner analysis
Comment 23 testify4 2012-10-29 11:09:33 UTC
(In reply to comment #18)
> I think we should try and use pressure sensitivity for this, rather than a
> timeout. We should be able to do this after the next X11 release.

Hi! It's probably worth checking this pressure sensitivity if the amount of work to carry out an experiment is acceptable. However, I tried to understand the problem in the attachment 227510 [details], and if my analysis is correct, the hot corner is just “brushed” (touched for a very short time), and then cursor is immediately moved away for it to real destinations: dash or window picker or... application widget. If we require some extra “pushing”, then I'm even more concerned about speed of window switching. (By the way, proposed behavior 2 in my mockup tries to address faster window switching).
Comment 24 drago01 2013-01-25 20:13:20 UTC
Review of attachment 201247 [details] [review]:

"I think we should try and use pressure sensitivity for this, rather than a
timeout. We should be able to do this after the next X11 release." <-- I agree with that.

A timeout also has the side effect of accidently triggering it by forgetting the mouse in the corner (like the message tray timeout in 3.6) so lets use pressure sensitivity instead.
Comment 25 drago01 2013-01-25 20:14:27 UTC
Review of attachment 202903 [details] [review]:

Same applies here (and we don't really support 3.0 anymore).
Comment 26 Matthias Clasen 2013-03-01 03:24:36 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > Do you have a URL (mailing list post or anything) about that feature?  I'm a
> > bit disconnected from X11 happenings these days.
> 
> I'm working on it. Check my GitHub page for the latest code dump.

Jasper, any update on this ? Would have to get this in before .91, I think.
Comment 27 Jasper St. Pierre (not reading bugmail) 2013-03-01 04:18:40 UTC
(In reply to comment #26)
> Jasper, any update on this ? Would have to get this in before .91, I think.

I'll try to work on it tomorrow. I wasn't planning on it for 3.8, but if it's a blocker, it's a blocker...
Comment 28 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:17:49 UTC
Created attachment 237752 [details] [review]
layout: Prevent going into negatives with the pressure

We capped each event to 15px of travel, but we didn't do the same
cap when subtracting events that were too old.
Comment 29 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:17:54 UTC
Created attachment 237753 [details] [review]
layout: Debounce triggering barriers

Ensure that the pointer leaves the barrier before we trigger again.

For the message tray case, this doesn't matter much, as the trigger
won't have any effect after the grab is taken, but in the overview
HotCorner case, this ensures that we don't trigger the overview
transition many times simply by holding pressure against the hot
corner, which is easy to do accidentally.
Comment 30 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:17:58 UTC
Created attachment 237754 [details] [review]
layout: Move the keybinding mode to the user of PressureBarrier

For the hot corner case, we want to have the pressure apply both in
and outside of the overview, so we need to move this to the user. At
the same time, use keybinding mode math that's more like what's used
in filterKeybinding.

While it may seem like an abuse of the KeyBindingMode API, it may
become more reasonable if one thinks of the pressure barrier as a
binding of sorts, just applied to the mouse. If a ButtonBinding API
was added to mutter, I think we'd use the existing KeyBindingMode
infastructure there as well.
Comment 31 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:02 UTC
Created attachment 237755 [details] [review]
layout: Move tray-specific event filtration to the user of PressureBarrier

For the HotCorner, we want to have different logic for tossing out
specific events based on the grabbed state, etc. so make us have
to pass in an event filter callback.
Comment 32 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:09 UTC
Created attachment 237756 [details] [review]
layout: Allow multiple barriers to contribute to a PressureBarrier

For the HotCorner case, we want to have to barriers both contribute
to the hot corner pressure, so we can't simply wrap the pressure
barrier.
Comment 33 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:13 UTC
Created attachment 237757 [details] [review]
layout: Construct the primary monitors's hot corner, too

This cleans up the code considerably, and makes it so that
one path creates all hot corners for all monitors. Why this
wasn't done originally, I have no clue...

The one complication is debouncing if the button and hot corner
are triggered in rapid succession, so we just move this tracking
to the overview.
Comment 34 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:17 UTC
Created attachment 237758 [details] [review]
layout: Pass the X/Y coordinates when constructing the corner

There's no guarantee that hot corners will have an actor in
the future -- they may be powered entirely by a barrier.
Comment 35 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:21 UTC
Created attachment 237759 [details] [review]
layout: Put two barriers near every single hot corner

This will make the hot corner easier to hit on multi-monitor
scenarios, and also gives us a convenient set of barriers to
key pressure off of.
Comment 36 Jasper St. Pierre (not reading bugmail) 2013-03-01 22:18:26 UTC
Created attachment 237760 [details] [review]
layout: Port the hot corner to pressure barriers
Comment 37 Diogo Campos 2013-03-02 03:36:28 UTC
Hey guys, what's up?

Following the reasoning of comments 18 and 24, I propose that the "overview" is activated in the same way that is activated the "message tray", but using the left and right sides of the screen.


It would work like this:

# With the mouse: "push" any part of the left or right side of the screen.

- This eliminates the endless complaints about how Gnome Shell requires intense movement of the mouse. Because it allows the user to activate the "overview" in a place nearest where your mouse (or "target") is.

# On the touch screen: slide your finger, from the outside to inside, on the left or right side of the screen.

- This gives the possibility (and flexibility) of the "overview" to be easily activated with either hand.

# With the keyboard: nothing changes.


Analyzing fondly, what do you think?

NOTE: I think this also closes Bug 694474.
Comment 38 drago01 2013-03-02 11:13:24 UTC
Review of attachment 237760 [details] [review]:

::: js/ui/layout.js
@@ +1136,3 @@
+    },
+
+    _setupFallbackCornerIfNeeded: function() {

I though about this a bit after our discussion from yesterday about xdnd ... 
What we could do here is to unconditionally create the actor and only connect to the event handlers if we don't have barrier support. This way the dnd handling would still work, while the overview will only be triggered using the barriers in that case.
Comment 39 drago01 2013-03-02 11:14:25 UTC
(In reply to comment #37)
> Hey guys, what's up?
> 
> Following the reasoning of comments 18 and 24, I propose that the "overview" is
> activated in the same way that is activated the "message tray", but using the
> left and right sides of the screen.
> 
> 
> It would work like this:
> 
> # With the mouse: "push" any part of the left or right side of the screen.
> 
> - This eliminates the endless complaints about how Gnome Shell requires intense
> movement of the mouse. Because it allows the user to activate the "overview" in
> a place nearest where your mouse (or "target") is.
> 
> # On the touch screen: slide your finger, from the outside to inside, on the
> left or right side of the screen.
> 
> - This gives the possibility (and flexibility) of the "overview" to be easily
> activated with either hand.
> 
> # With the keyboard: nothing changes.
> 
> 
> Analyzing fondly, what do you think?

This does not work with multiple monitors.
Comment 40 Jasper St. Pierre (not reading bugmail) 2013-03-02 11:35:52 UTC
(In reply to comment #38)
> Review of attachment 237760 [details] [review]:
> 
> ::: js/ui/layout.js
> @@ +1136,3 @@
> +    },
> +
> +    _setupFallbackCornerIfNeeded: function() {
> 
> I though about this a bit after our discussion from yesterday about xdnd ... 
> What we could do here is to unconditionally create the actor and only connect
> to the event handlers if we don't have barrier support. This way the dnd
> handling would still work, while the overview will only be triggered using the
> barriers in that case.

I'm confused how unconditionally creating the actor will affect anything here, if we don't connect any signals to it. Clearly people should be able to open the overview with XDnD with pressure, not just hovering over the button.
Comment 41 drago01 2013-03-02 13:16:12 UTC
(In reply to comment #40)
> (In reply to comment #38)
> > Review of attachment 237760 [details] [review] [details]:
> > 
> > ::: js/ui/layout.js
> > @@ +1136,3 @@
> > +    },
> > +
> > +    _setupFallbackCornerIfNeeded: function() {
> > 
> > I though about this a bit after our discussion from yesterday about xdnd ... 
> > What we could do here is to unconditionally create the actor and only connect
> > to the event handlers if we don't have barrier support. This way the dnd
> > handling would still work, while the overview will only be triggered using the
> > barriers in that case.
> 
> I'm confused how unconditionally creating the actor will affect anything here,
> if we don't connect any signals to it. Clearly people should be able to open
> the overview with XDnD with pressure, not just hovering over the button.

If you want to use the barriers for XDND as well you'd have to implement the enter-event + flag thing we talked about on IRC yesterday.
Comment 42 Diogo Campos 2013-03-03 01:15:41 UTC
(In reply to comment #39)
> 
> This does not work with multiple monitors.

Hi, Drago01, thanks for the attention.

I never used Gnome Shell with multiple monitors, so I have a couple of questions (in order to improve my suggestion):

1- Why my suggestion doesn't work with multiple monitors?

2- Why the hot corner works with multiple monitors?
Comment 43 drago01 2013-03-03 08:36:54 UTC
(In reply to comment #42)
> (In reply to comment #39)
> > 
> > This does not work with multiple monitors.
> 
> Hi, Drago01, thanks for the attention.
> 
> I never used Gnome Shell with multiple monitors, so I have a couple of
> questions (in order to improve my suggestion):
> 
> 1- Why my suggestion doesn't work with multiple monitors?

Because it conflicts with moving the pointer between monitors.
If you want to move it to the left you'd open the overview ... that be really annoying.

> 2- Why the hot corner works with multiple monitors?

There is no reason why it shouldn't work.
Comment 44 Jasper St. Pierre (not reading bugmail) 2013-03-03 08:49:28 UTC
There's also potential issues with scrollbars on screen edges -- throwing the pointer to reach the scrollbar is fairly common, so we shouldn't open the overview there either.
Comment 45 Diogo Campos 2013-03-03 12:25:42 UTC
(In reply to comment #43)
> Because it conflicts with moving the pointer between monitors.
(In reply to comment #44)
> There's also potential issues with scrollbars on screen edges

I was thinking in something with "virtual" edges, like this:

#--------------------|--------------------#
#                              |                              #
#              1              |               2             #
#                              |                              #
#--------------------|--------------------#
#                              |                              #
#             3               |              4              #
#                              |                              #
#--------------------|--------------------#


* "number" = monitor.
* "lines" = physical division between monitors.
* "#" = pressure area.

Do you think these two problems persist in this scenario?
Comment 46 Diogo Campos 2013-03-03 12:36:47 UTC
(In reply to comment #45)
> #--------------------|--------------------#
> #                              |                              #
> #              1              |               2             #
> #                              |                              #
> #--------------------|--------------------#
> #                              |                              #
> #             3               |              4              #
> #                              |                              #
> #--------------------|--------------------#

Sorry, Bugzilla messed up my drawing.
The right drawing is attached.
Comment 47 Diogo Campos 2013-03-03 12:44:47 UTC
Created attachment 237874 [details]
Activate overview with multiple monitor.

Sorry for the mess.
Comment 48 Jasper St. Pierre (not reading bugmail) 2013-03-03 20:54:12 UTC
Created attachment 237900 [details] [review]
layout: Extract toggling overview logic into one function

Since barriers will continue to get events even when another client
is grabbed, we have to track the current status of XDnD to determine
whether to take a grab ourselves or not.
Comment 49 Jasper St. Pierre (not reading bugmail) 2013-03-03 20:54:59 UTC
Created attachment 237901 [details] [review]
layout: Put two barriers near every single hot corner

This will make the hot corner easier to hit on multi-monitor
scenarios, and also gives us a convenient set of barriers to
key pressure off of.
Comment 50 Jasper St. Pierre (not reading bugmail) 2013-03-03 20:55:11 UTC
Created attachment 237902 [details] [review]
layout: Don't use the corner's position for positioning ripples

The corner may not be there in the future.
Comment 51 Jasper St. Pierre (not reading bugmail) 2013-03-03 20:55:30 UTC
Created attachment 237903 [details] [review]
layout: Port the hot corner to pressure barriers

If the X server supports the new barrier features, we should
trigger the overview hot corner with a pressure barrier as well.



Fix XDnD, and add back ripples when triggering barriers.
Comment 52 drago01 2013-03-03 22:35:05 UTC
*** Bug 638898 has been marked as a duplicate of this bug. ***
Comment 53 drago01 2013-03-04 20:00:13 UTC
Review of attachment 237752 [details] [review]:

OK.
Comment 54 drago01 2013-03-04 20:01:39 UTC
Review of attachment 237753 [details] [review]:

Makes sense.
Comment 55 drago01 2013-03-04 20:04:16 UTC
Review of attachment 237754 [details] [review]:

I am not sure I buy the "barrier is a keybinding" thing but reusing what we already have rather the duplicating it makes sense.
Comment 56 drago01 2013-03-04 20:06:43 UTC
Review of attachment 237755 [details] [review]:

OK.
Comment 57 drago01 2013-03-04 20:11:03 UTC
Review of attachment 237756 [details] [review]:

OK.
Comment 58 drago01 2013-03-04 20:15:01 UTC
Review of attachment 237757 [details] [review]:

Yeah this makes a lot of sense ... no idea why didn't do that originally either.
Comment 59 drago01 2013-03-04 20:20:24 UTC
Review of attachment 237900 [details] [review]:

You have to call hideTemporarily when entering the overview using showTemporarily ... just calling toggle() won't do that. (it will just call hide).
Check for this._shownTemporarily in toggle() and call hideTemporarily when appropriate.
Comment 60 drago01 2013-03-04 20:22:14 UTC
Review of attachment 237901 [details] [review]:

OK.
Comment 61 drago01 2013-03-04 20:22:56 UTC
Review of attachment 237902 [details] [review]:

OK.
Comment 62 drago01 2013-03-04 20:27:14 UTC
Review of attachment 237903 [details] [review]:

::: js/ui/layout.js
@@ +426,3 @@
     _setupTrayPressure: function() {
+        this._trayPressure = new PressureBarrier(HOT_CORNER_PRESSURE_THRESHOLD,
+                                                 HOT_CORNER_PRESSURE_TIMEOUT,

This is backwards why are you using the HOT_CORNER values for the tray?

@@ +1115,2 @@
+        this._pressureBarrier = new PressureBarrier(MESSAGE_TRAY_PRESSURE_THRESHOLD,
+                                                    MESSAGE_TRAY_PRESSURE_TIMEOUT,

And now the tray values for the hot corner ?

@@ +1175,3 @@
+            if (Clutter.get_default_text_direction() == Clutter.TextDirection.RTL) {
+                this._corner.set_position(this.actor.width - this._corner.width, 0);
+                this.actor.set_anchor_point_from_gravity(Clutter.Gravity.NORTH_EAST);

We should aim to get rid of deprecated code, not use deprecated methods in new code, just use pivot-point instead.
Comment 63 Jasper St. Pierre (not reading bugmail) 2013-03-04 20:29:30 UTC
(In reply to comment #62)
> Review of attachment 237903 [details] [review]:
> 
> ::: js/ui/layout.js
> @@ +426,3 @@
>      _setupTrayPressure: function() {
> +        this._trayPressure = new
> PressureBarrier(HOT_CORNER_PRESSURE_THRESHOLD,
> +                                                 HOT_CORNER_PRESSURE_TIMEOUT,
> 
> This is backwards why are you using the HOT_CORNER values for the tray?
> 
> @@ +1115,2 @@
> +        this._pressureBarrier = new
> PressureBarrier(MESSAGE_TRAY_PRESSURE_THRESHOLD,
> +                                                   
> MESSAGE_TRAY_PRESSURE_TIMEOUT,
> 
> And now the tray values for the hot corner ?

... whoops! I initially just used the message tray values, and then did a find and replace, and replaced the wrong values.

> @@ +1175,3 @@
> +            if (Clutter.get_default_text_direction() ==
> Clutter.TextDirection.RTL) {
> +                this._corner.set_position(this.actor.width -
> this._corner.width, 0);
> +               
> this.actor.set_anchor_point_from_gravity(Clutter.Gravity.NORTH_EAST);
> 
> We should aim to get rid of deprecated code, not use deprecated methods in new
> code, just use pivot-point instead.

This isn't new code. It's code I copy/pasted from old the corner construction code. If we want to fix the deprecation warnings, that's another patch.
Comment 64 drago01 2013-03-04 20:31:33 UTC
> This isn't new code. It's code I copy/pasted from old the corner construction
> code.

OK fair enough.

> If we want to fix the deprecation warnings, that's another patch.

There are no warnings because this is JS ...
Comment 65 Jasper St. Pierre (not reading bugmail) 2013-03-04 20:52:21 UTC
Pushing the ACN patches to clean up for now.
Comment 66 Jasper St. Pierre (not reading bugmail) 2013-03-04 22:03:57 UTC
Attachment 237901 [details] pushed as 62bf08d - layout: Put two barriers near every single hot corner
Attachment 237902 [details] pushed as 7be1fe0 - layout: Don't use the corner's position for positioning ripples


Let's push these as well. Last two patches appearing soon.
Comment 67 Jasper St. Pierre (not reading bugmail) 2013-03-04 22:39:18 UTC
Created attachment 238050 [details] [review]
overview: Try to do the right thing related to XDnD

Rather than expose a dizzying array of methods related to managing
state that require infecting every user of the overview methods, try
to do the sensible and smart thing internally. Now, the overview
itself tracks when XDND drags start, and simply calling show, hide or
toggle while an XDnD drag is in effect will show the overview, and
will only take the grab until after the XDND drag ends.



This should be an equivalent cleanup that I find much better.
Comment 68 Jasper St. Pierre (not reading bugmail) 2013-03-04 22:39:25 UTC
Created attachment 238051 [details] [review]
layout: Port the hot corner to pressure barriers

If the X server supports the new barrier features, we should
trigger the overview hot corner with a pressure barrier as well.
Comment 69 drago01 2013-03-04 22:44:23 UTC
Review of attachment 238050 [details] [review]:

::: js/ui/overview.js
@@ +364,3 @@
         // the overview
+        if (this._shown)
+            this.hide();

Why did you remove the workspace switch? The "go back to where we started" implies the same workspace as well (you can switch workspaces during dnd).
Comment 70 drago01 2013-03-04 22:45:34 UTC
Review of attachment 238051 [details] [review]:

LG.
Comment 71 Jasper St. Pierre (not reading bugmail) 2013-03-04 22:48:09 UTC
Created attachment 238056 [details] [review]
overview: Try to do the right thing related to XDnD

Rather than expose a dizzying array of methods related to managing
state that require infecting every user of the overview methods, try
to do the sensible and smart thing internally. Now, the overview
itself tracks when XDND drags start, and simply calling show, hide or
toggle while an XDnD drag is in effect will show the overview, and
will only take the grab until after the XDND drag ends.
Comment 72 drago01 2013-03-04 22:49:52 UTC
Review of attachment 238056 [details] [review]:

LG.
Comment 73 Jasper St. Pierre (not reading bugmail) 2013-03-04 22:54:46 UTC
Attachment 238051 [details] pushed as 26966b2 - layout: Port the hot corner to pressure barriers
Attachment 238056 [details] pushed as 36a7429 - overview: Try to do the right thing related to XDnD