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 728343 - St: remove hover state if reactive disabled
St: remove hover state if reactive disabled
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: st
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-04-16 14:00 UTC by Carlos Soriano
Modified: 2014-04-28 23:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
St: remove hover state if reactive disabled (1.31 KB, patch)
2014-04-16 14:00 UTC, Carlos Soriano
reviewed Details | Review
St: remove hover state if reactive disabled (1.42 KB, patch)
2014-04-27 14:03 UTC, Carlos Soriano
needs-work Details | Review
St: remove hover state if reactive disabled (1.47 KB, patch)
2014-04-28 22:33 UTC, Carlos Soriano
reviewed Details | Review
St: remove hover state if reactive disabled (1.46 KB, patch)
2014-04-28 22:44 UTC, Carlos Soriano
committed Details | Review

Description Carlos Soriano 2014-04-16 14:00:41 UTC
The hover state of a widget can become persistent if
the widget becomes reactive while a pointer grab.
To avoid that, remove hover state if the reactive property
is disabled.
Comment 1 Carlos Soriano 2014-04-16 14:00:45 UTC
Created attachment 274457 [details] [review]
St: remove hover state if reactive disabled
Comment 2 Florian Müllner 2014-04-17 09:38:42 UTC
Review of attachment 274457 [details] [review]:

::: src/st/st-widget.c
@@ +1449,3 @@
+      // Remove hover state to avoid persistent hover state if
+      // the widget becomes not reactive while a pointer grab
+      st_widget_set_hover (widget, FALSE);

I'd prefer updating sync_hover() to take reactivity into account and then call it here if track_hover is set. That way
 1. calling sync_hover() will work as expected for non-reactive actors
 2. the hover state will be correct after making an actor reactive
Comment 3 Carlos Soriano 2014-04-27 14:03:35 UTC
Created attachment 275262 [details] [review]
St: remove hover state if reactive disabled

The hover state of a widget can become persistent if
the widget becomes reactive while a pointer grab.
To avoid that, remove hover state if the reactive property
is disabled.
Comment 4 Florian Müllner 2014-04-28 22:14:07 UTC
Review of attachment 275262 [details] [review]:

::: src/st/st-widget.c
@@ +1445,3 @@
     st_widget_add_style_pseudo_class (widget, "insensitive");
+
+  st_widget_sync_hover(widget);

When StWidget:track-hover is not set, we should not interfere with a manually set hover state.
Comment 5 Carlos Soriano 2014-04-28 22:27:34 UTC
(In reply to comment #4)
> Review of attachment 275262 [details] [review]:
> 
> ::: src/st/st-widget.c
> @@ +1445,3 @@
>      st_widget_add_style_pseudo_class (widget, "insensitive");
> +
> +  st_widget_sync_hover(widget);
> 
> When StWidget:track-hover is not set, we should not interfere with a manually
> set hover state.

Right, you already said it before. Sorry, I forgot to add it.
Comment 6 Carlos Soriano 2014-04-28 22:33:07 UTC
Created attachment 275390 [details] [review]
St: remove hover state if reactive disabled

The hover state of a widget can become persistent if
the widget becomes reactive while a pointer grab.
To avoid that, remove hover state if the reactive property
is disabled.
Comment 7 Florian Müllner 2014-04-28 22:35:48 UTC
Review of attachment 275390 [details] [review]:

Almost ...

::: src/st/st-widget.c
@@ +1445,3 @@
     st_widget_add_style_pseudo_class (widget, "insensitive");
+
+  if (st_widget_get_track_hover(widget))

Just access the struct member directly.
Comment 8 Carlos Soriano 2014-04-28 22:44:21 UTC
Created attachment 275391 [details] [review]
St: remove hover state if reactive disabled

The hover state of a widget can become persistent if
the widget becomes reactive while a pointer grab.
To avoid that, remove hover state if the reactive property
is disabled.
Comment 9 Carlos Soriano 2014-04-28 22:45:39 UTC
(In reply to comment #7)
> Review of attachment 275390 [details] [review]:
> 
> Almost ...
> 
> ::: src/st/st-widget.c
> @@ +1445,3 @@
>      st_widget_add_style_pseudo_class (widget, "insensitive");
> +
> +  if (st_widget_get_track_hover(widget))
> 
> Just access the struct member directly.

Don't I need g_return_val_if_fail (ST_IS_WIDGET (widget), FALSE); then?
Comment 10 Florian Müllner 2014-04-28 22:50:22 UTC
(In reply to comment #9)
> Don't I need g_return_val_if_fail (ST_IS_WIDGET (widget), FALSE); then?

No. That is input validation in a public function. You don't need this in an internal function where you know that the object type is correct.
Comment 11 Florian Müllner 2014-04-28 22:50:36 UTC
Review of attachment 275391 [details] [review]:

Yes.
Comment 12 Carlos Soriano 2014-04-28 23:04:18 UTC
Attachment 275391 [details] pushed as 103027a - St: remove hover state if reactive disabled