#4221 closed defect (fixed)
Hover effect stays after changing zoom views with keyboard shortcuts
Reported by: | erikos | Owned by: | erikos |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | Sugar | Version: | Unspecified |
Severity: | Major | Keywords: | regression, r+ |
Cc: | manuq | Distribution/OS: | OLPC |
Bug Status: | Assigned |
Description
Build: os11, XO-1.5
Steps to reproduce:
- use the zoom level keys on the hw keyboard to switch the views
---> the grey rectangle that is used to indicate the hover state of the icon is shown for the owner icon, the cursor is not over the owner icon
Attachments (3)
Change History (19)
comment:1 Changed 11 years ago by manuq
- Keywords triage removed
- Priority changed from Unspecified by Maintainer to Low
- Severity changed from Unspecified to Minor
comment:2 Changed 11 years ago by erikos
- Bug Status changed from Unconfirmed to Assigned
- Cc manuq removed
- Keywords regression added
- Milestone changed from Unspecified by Release Team to 0.98
- Priority changed from Low to Normal
- Severity changed from Minor to Major
- Status changed from new to assigned
comment:3 Changed 11 years ago by manuq
- Cc erikos added
- Keywords r? added
comment:4 Changed 11 years ago by manuq
Updated the attached patch. However it solves another issue: the hover feedback (grey rectangle) is displayed at Sugar startup.
The issue described in this ticket seems related to palettes. Here is the traceback after pressing the zoom level keys, having the mouse pointer over the owner icon:
Traceback (most recent call last): File "/home/manuq/prog/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 565, in _mouse_slow_cb self._palette_do_popup() File "/home/manuq/prog/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 580, in _palette_do_popup self.popup(immediate=immediate) File "/home/manuq/prog/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palette.py", line 206, in popup PaletteWindow.popup(self, immediate) File "/home/manuq/prog/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 626, in popup self._widget.set_transient_for(self._invoker.get_toplevel()) File "/home/manuq/prog/sugar-build/install/lib64/python2.7/site-packages/gi/types.py", line 47, in function return info.invoke(*args, **kwargs) TypeError: argument parent: Expected Gtk.Window, but got jarabe.desktop.homebox.HomeBox
comment:5 Changed 11 years ago by manuq
- Keywords r? removed
The patch went in but as said above fixes another issue. Removing r?
comment:6 Changed 10 years ago by erikos
Actually this is easy to reproduce:
- start the XO
- without any other interaction use the zoom keys to switch between the views
---> the grey feedback is shown because the cursor is over the XO icon, even after moving out the cursor we do get the grey feedback now when using the zoom keys
comment:7 Changed 10 years ago by manuq
- Summary changed from Zooming Views: the XO icon does show the grey hover colour to Hover effect stays after changing zoom views with keyboard shortcuts
Also reporded as #4386 . Making title more clear.
comment:8 Changed 10 years ago by erikos
There are three issues that I can see (when the mouse pointer is over an XO icon and we use the keys for switching between views):
a) the hover feedback is seen in the animation (transition box)
b) the hover feedback does not go away in the originating icon we had the mouse over
c) sometimes the transition icon is cropped when going from the Group view to the Home view (need clear steps to reproduce this)
comment:9 Changed 10 years ago by erikos
The following patch covers point (a):
diff --git a/src/jarabe/desktop/transitionbox.py b/src/jarabe/desktop/transitionbox.py index b124b14..3f8650a 100644 --- a/src/jarabe/desktop/transitionbox.py +++ b/src/jarabe/desktop/transitionbox.py @@ -18,9 +18,9 @@ from gi.repository import GObject from sugar3.graphics import style from sugar3.graphics import animator +from sugar3.graphics.icon import Icon from jarabe.model.buddy import get_owner_instance -from jarabe.view.buddyicon import BuddyIcon from jarabe.desktop.viewcontainer import ViewContainer from jarabe.desktop.favoriteslayout import SpreadLayout @@ -49,8 +49,10 @@ class TransitionBox(ViewContainer): layout = SpreadLayout() # Round off icon size to an even number to ensure that the icon - self._owner_icon = BuddyIcon(buddy=get_owner_instance(), - pixel_size=style.XLARGE_ICON_SIZE & ~1) + owner = get_owner_instance() + self._owner_icon = Icon(icon_name='computer-xo', + xo_color=owner.get_color(), + pixel_size=style.XLARGE_ICON_SIZE & ~1) ViewContainer.__init__(self, layout, self._owner_icon) self._animator = animator.Animator(0.3)
The icon in the transition box does not need to be a canvas icon that reacts to input. It can be an icon only. That helps us to not trigger any visual hover feedback for that icon.
comment:10 Changed 10 years ago by erikos
For point b:
When we use the view keys to toggle the view we do not get a popdown or a leave event for the canvas icon in order to clear the state. In the logs we do get:
1363257990.450653 DEBUG root: _key_pressed_cb: 68 0 F2 1363257990.456624 DEBUG root: new zoom level: 1 Traceback (most recent call last): File "/home/erikos/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/animator.py", line 107, in _next_frame_cb animation.do_frame(current_time, self._duration, self._easing) File "/home/erikos/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/animator.py", line 148, in do_frame self.next_frame(frame) File "/home/erikos/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 739, in next_frame self._palette.popup(immediate=True) File "/home/erikos/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palette.py", line 207, in popup PaletteWindow.popup(self, immediate) File "/home/erikos/sugar-build/install/lib/python2.7/site-packages/sugar3/graphics/palettewindow.py", line 626, in popup self._widget.set_transient_for(self._invoker.get_toplevel()) File "/usr/lib64/python2.7/site-packages/gi/types.py", line 47, in function return info.invoke(*args, **kwargs) TypeError: argument parent: Expected Gtk.Window, but got jarabe.desktop.homebox.HomeBox
I guess we can cover the case where we can not call "set_transient_for" for the Palette and in that case emit a 'popdown' signal. This can be handled by the canvas icon to clear the state.
diff --git a/src/sugar3/graphics/palettewindow.py b/src/sugar3/graphics/palettewindow.py index c48ae55..524fb03 100644 --- a/src/sugar3/graphics/palettewindow.py +++ b/src/sugar3/graphics/palettewindow.py @@ -623,7 +623,11 @@ class PaletteWindow(GObject.GObject): self._alignment = self._invoker.get_alignment(full_size_request) self.update_position() - self._widget.set_transient_for(self._invoker.get_toplevel()) + try: + self._widget.set_transient_for(self._invoker.get_toplevel()) + except TypeEror: + self.emit('popdown') + return self._popdown_anim.stop()
(we should add a comment here in the code explaining the whys)
comment:11 Changed 10 years ago by erikos
- Cc manuq added; erikos removed
comment:12 Changed 10 years ago by erikos
- Keywords r? added
Changed 10 years ago by erikos
Palette: handle the case where setting the transient window does fail
comment:13 Changed 10 years ago by erikos
(c) I am having issues reproducing atm (as well without the patches from above). But I have already seen it several times. Extra points for someone which has steps to reproduce.
comment:14 Changed 10 years ago by manuq
- Keywords r+ added; r? removed
Excellent Simon. Yes both fixes look right to me, and the comment added in the toolkit one is clever.
I can see c) is reproduceable if the transition icon is a CanvasIcon. Your patch for shell fixes that point too.
Please push.
comment:15 Changed 10 years ago by erikos
- Resolution set to fixed
- Status changed from assigned to closed
shell patch: 2577166a9ec64ee2a16abf61397bd75bc279984d fac83b0c892eb522d824a8f5fcf3f59b77aedf1c
toolkit patch: b3048112d67db0c4901c49bf366d43e55d9b2fd5 bc9eab06e2db844b415b8a36647c34959fe796d7
Ok, sounds good if (c) is fixed as well. If we should see it again we can open a separate ticket for it.
I would say this has a bit higher priority since it is a regression. The user will notice this change in behaviour. Moving up a bit.