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 773119 - Floating bar not hiding on hover with two or more tabs
Floating bar not hiding on hover with two or more tabs
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Views: All
master
Other Linux
: Normal normal
: ---
Assigned To: Ernestas Kulik
Nautilus Maintainers
: 778007 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-10-17 18:45 UTC by gedgon
Modified: 2017-02-01 06:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
floating-bar: fix hide on hover with notebook visible (1.86 KB, patch)
2016-10-18 16:29 UTC, Ernestas Kulik
none Details | Review
floating-bar: fix hide on hover with notebook visible (1.94 KB, patch)
2016-10-28 14:40 UTC, Ernestas Kulik
committed Details | Review

Description gedgon 2016-10-17 18:45:25 UTC
Reproducible steps:
1. Press Ctrl+N
2. Select any item in Icon or List View
3. Move mouse cursor over a floating bar.
Comment 1 gedgon 2016-10-17 18:50:36 UTC
Sorry. Step 1 is Ctrl+T (new tab), of course.
Comment 2 Ernestas Kulik 2016-10-17 19:12:28 UTC
Thanks for reporting this. I will take a look when I find a freer moment tomorrow.
Comment 3 Ernestas Kulik 2016-10-18 16:29:45 UTC
Created attachment 337950 [details] [review]
floating-bar: fix hide on hover with notebook visible

When the notebook is visible, the pointer intersection with the floating
bar check fails. That is due to how the floating bar bounds are
calculated - the upper y bound becomes smaller than the lower y bound.
That can be fixed by calculating the upper bound relative to the lower
one.
Comment 4 Ernestas Kulik 2016-10-18 16:32:58 UTC
If I am being honest, I have no idea why my fix works. It appears that GTK+ and GDK have conflicting ideas about positioning.
Comment 5 gedgon 2016-10-18 16:39:53 UTC
(In reply to Ernestas Kulik from comment #4)
> If I am being honest, I have no idea why my fix works. It appears that GTK+
> and GDK have conflicting ideas about positioning.

Works like a charm. Thanks!
Comment 6 Carlos Soriano 2016-10-24 16:26:38 UTC
Review of attachment 337950 [details] [review]:

Would be good to explain the reason in the commit message.

The widget windor of the GtkOverlay has y = 0 when there are no tabs, and y = 37 (depending on theme and font size) when there are tabs.
However we are not taking that into account and instead just setting the parents allocation as a upper limit.
Rule of thumb is when using positions always take into account the relative position of the parent. But I think part of this code is mine so I will not be grumpy about it :D

Once you explain that in the commit message, just commit it.
Comment 7 Carlos Soriano 2016-10-24 16:27:13 UTC
I forgot to mention, the code of the patch looks perfectly good, thanks! :)
Comment 8 Ernestas Kulik 2016-10-24 18:55:40 UTC
(In reply to Carlos Soriano from comment #6)
> Review of attachment 337950 [details] [review] [review]:
> 
> Would be good to explain the reason in the commit message.
> 
> The widget windor of the GtkOverlay has y = 0 when there are no tabs, and y
> = 37 (depending on theme and font size) when there are tabs.
> However we are not taking that into account and instead just setting the
> parents allocation as a upper limit.
> Rule of thumb is when using positions always take into account the relative
> position of the parent. But I think part of this code is mine so I will not
> be grumpy about it :D
> 
> Once you explain that in the commit message, just commit it.

The code actually made sense to me. The lower limt is set as the y coordinate of the floating bar /within/ the parent overlay and the upper limit is set as the height /of/ the parent overlay, no?
Comment 9 Carlos Soriano 2016-10-24 19:06:14 UTC
(In reply to Carlos Soriano from comment #7)
> I forgot to mention, the code of the patch looks perfectly good, thanks! :)

Oh that's not explicity, but rather duplication, which is wrong. This is something documented in the API of glib/gtk+, there is not need to document it, in the same way you don't document g_file_rename or so saying "rename the file".

Comments should always be the last resort (as opposed of what some universities teach).

What it's not clear here is that you are unescheduling something that is already unescheduled because you return g_source_remove. So you clarify "it's just for setting the id to 0 in the priv data...".
Comment 10 Ernestas Kulik 2016-10-24 19:10:30 UTC
(In reply to Carlos Soriano from comment #9)
> (In reply to Carlos Soriano from comment #7)
> > I forgot to mention, the code of the patch looks perfectly good, thanks! :)
> 
> Oh that's not explicity, but rather duplication, which is wrong. This is
> something documented in the API of glib/gtk+, there is not need to document
> it, in the same way you don't document g_file_rename or so saying "rename
> the file".
> 
> Comments should always be the last resort (as opposed of what some
> universities teach).
> 
> What it's not clear here is that you are unescheduling something that is
> already unescheduled because you return g_source_remove. So you clarify
> "it's just for setting the id to 0 in the priv data...".

Wait… This is the wrong bug. :D
Comment 11 Carlos Soriano 2016-10-24 19:26:02 UTC
wtf how this happened?
Comment 12 Ernestas Kulik 2016-10-28 14:40:12 UTC
Created attachment 338726 [details] [review]
floating-bar: fix hide on hover with notebook visible

When the notebook is visible, the pointer intersection with the floating
bar check fails. That is due to how the floating bar bounds are
calculated - the offset of the overlay when tabs are visible is not
taken into account, allowing the upper y bound to become smaller than
the lower bound. That can be fixed by calculating the upper y bound
relative to the lower one.
Comment 13 Carlos Soriano 2016-11-03 10:04:21 UTC
Review of attachment 338726 [details] [review]:

YEP
Comment 14 Ernestas Kulik 2016-11-03 11:21:15 UTC
Attachment 338726 [details] pushed as 0880338 - floating-bar: fix hide on hover with notebook visible
Comment 15 Ernestas Kulik 2017-02-01 06:44:34 UTC
*** Bug 778007 has been marked as a duplicate of this bug. ***