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 624940 - lookingGlass improvements
lookingGlass improvements
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-07-21 15:55 UTC by Dan Winship
Modified: 2010-08-23 17:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[lg] split inspector event handlers out into separate functions (4.63 KB, patch)
2010-07-21 15:56 UTC, Dan Winship
committed Details | Review
[lg] Use scroll wheel in inspector to select parent actors (4.28 KB, patch)
2010-07-21 15:56 UTC, Dan Winship
committed Details | Review
[lg] Close inspector on Esc (1.84 KB, patch)
2010-07-21 15:56 UTC, Dan Winship
committed Details | Review
[lg] fix queue_relayout warnings (2.43 KB, patch)
2010-07-21 15:56 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2010-07-21 15:55:57 UTC
I wanted to inspect an actor that had no padding or spacing, but could
only select its children. So I wrote a patch that lets you use the
scroll wheel to select higher or lower levels in the higherarchy.
(Scroll up selects successively-higher parents, scroll-down selects
back toward the child you started from.)

After doing that it occurred to me that maybe it would be better to have
it be based on Z order rather than hierarchy (ie, by hiding some actors
around the pick() call so as to end up selecting the actors behind them).
Could do that too...

The first patch is just a reorg of existing code, and could be squashed
with the scrollwheel patch. The third patch makes "Esc" DTRT. The fourth
fixes the queue_relayout warnings, because they were annoying me.
Comment 1 Dan Winship 2010-07-21 15:56:02 UTC
Created attachment 166298 [details] [review]
[lg] split inspector event handlers out into separate functions
Comment 2 Dan Winship 2010-07-21 15:56:07 UTC
Created attachment 166299 [details] [review]
[lg] Use scroll wheel in inspector to select parent actors

scroll-up selects the parent actor, then grandparent, etc. scroll-down
moves the selection back towards the frontmost actor.
Comment 3 Dan Winship 2010-07-21 15:56:10 UTC
Created attachment 166300 [details] [review]
[lg] Close inspector on Esc
Comment 4 Dan Winship 2010-07-21 15:56:14 UTC
Created attachment 166301 [details] [review]
[lg] fix queue_relayout warnings
Comment 5 William Jon McCann 2010-07-21 15:59:01 UTC
Don't know if this is at all applicable but just saw this go by and wanted to mention that Inkscape uses alt+click to successively drill down the z-order for selections.  Don't know if that helps at all.
Comment 6 Colin Walters 2010-07-22 00:35:15 UTC
Review of attachment 166298 [details] [review]:

Looks good.
Comment 7 Colin Walters 2010-07-22 00:42:11 UTC
Review of attachment 166299 [details] [review]:

::: js/ui/lookingGlass.js
@@ +425,3 @@
+            // select parent
+        switch (event.get_scroll_direction()) {
+    _onScrollEvent: function (actor, event) {

I find the split between _target and _pointerTarget confusing; the way you have this coded, you can't move the mouse between going up and down, right?  That seems like it'd require very steady hands.  Maybe instead have UP set an explicit "searching" mode, and require a click to exit it?
Comment 8 Colin Walters 2010-07-22 00:42:47 UTC
Review of attachment 166300 [details] [review]:

Looks OK.
Comment 9 Colin Walters 2010-07-22 00:43:53 UTC
Review of attachment 166301 [details] [review]:

Looks fine.
Comment 10 Dan Winship 2010-07-22 01:15:36 UTC
(In reply to comment #7)
> I find the split between _target and _pointerTarget confusing; the way you have
> this coded, you can't move the mouse between going up and down, right?

No. I guess I should have added a comment.

_pointerTarget is the actor immediately under the pointer, and _target is the actor currently selected by the inspector (which is always either _pointerTarget or one of its ancestors). But the motion-event handler was tweaked so that it doesn't set _target on every event; it only changes _target if _pointerTarget changes. So, eg, if you hover over the "Activities" StLabel, and scroll up, it will select its parent (the 'panelActivities' StClickable), and that will stay selected until you move the pointer outside the StLabel.
Comment 11 Dan Winship 2010-08-23 17:06:09 UTC
(belatedly) added a comment and pushed

Attachment 166298 [details] pushed as 1a50b94 - [lg] split inspector event handlers out into separate functions
Attachment 166299 [details] pushed as 09edf4b - [lg] Use scroll wheel in inspector to select parent actors
Attachment 166300 [details] pushed as 083b1c9 - [lg] Close inspector on Esc
Attachment 166301 [details] pushed as 957b3b6 - [lg] fix queue_relayout warnings