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 657315 - Highlight application windows when hovering launcher icon
Highlight application windows when hovering launcher icon
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: general
3.24.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 791364 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-08-25 10:06 UTC by Julien Olivier
Modified: 2021-07-05 14:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Transparent windows (626.84 KB, image/png)
2011-12-18 00:50 UTC, Seif Lotfy
  Details
dash: properly restore item label on popup close (2.94 KB, patch)
2012-11-03 16:50 UTC, Stéphane Démurget
reviewed Details | Review
overview: highlight windows on launcher hover (17.40 KB, patch)
2012-11-03 16:52 UTC, Stéphane Démurget
needs-work Details | Review
dash: properly restore item label on popup close (3.07 KB, patch)
2012-11-03 19:26 UTC, Stéphane Démurget
reviewed Details | Review
dash: rename a local variable for clarity (2.29 KB, patch)
2012-11-03 19:26 UTC, Stéphane Démurget
accepted-commit_now Details | Review
overview: highlight windows on launcher hover (16.04 KB, patch)
2012-11-03 19:26 UTC, Stéphane Démurget
needs-work Details | Review
dash: properly restore item label on popup close (3.20 KB, patch)
2012-11-03 21:21 UTC, Stéphane Démurget
none Details | Review
dash: simplify app retrieval from AppWellIcon (1.18 KB, patch)
2012-11-03 21:21 UTC, Stéphane Démurget
none Details | Review
overview: highlight windows on launcher hover (15.33 KB, patch)
2012-11-03 21:22 UTC, Stéphane Démurget
none Details | Review
screenshot (658.51 KB, image/png)
2012-11-05 10:37 UTC, Allan Day
  Details
screenshot (637.13 KB, image/png)
2012-11-05 10:38 UTC, Allan Day
  Details
dash: properly restore item label on popup close (3.18 KB, patch)
2012-11-06 21:17 UTC, Stéphane Démurget
committed Details | Review
dash: rename a local variable for clarity (2.49 KB, patch)
2012-11-06 21:17 UTC, Stéphane Démurget
committed Details | Review
dash: simplify app retrieval from AppWellIcon (1.18 KB, patch)
2012-11-06 21:17 UTC, Stéphane Démurget
committed Details | Review
overview: highlight windows on launcher hover (16.71 KB, patch)
2012-11-06 21:17 UTC, Stéphane Démurget
needs-work Details | Review

Description Julien Olivier 2011-08-25 10:06:13 UTC
When in the overview, if you move the mouse cursor over one of the application launchers in the dash, all this application's windows should be highlighted both in the overview and in the workspace previews on the right.

I think it could help easily understand whether or not you already have windows opened for this application, and where they are. It could also help differentiate the windows in the overview (sometimes the thumbnails aren't precise enough to easily know which thumbnail belongs to which application).
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-12-15 00:24:34 UTC
We used to do this behavior, but it was removed because it looked too busy.
Comment 2 Allan Day 2011-12-15 09:01:44 UTC
(In reply to comment #1)
> We used to do this behavior, but it was removed because it looked too busy.

Oh really? CC'ing Jon to get some history.
Comment 3 Florian Müllner 2011-12-15 11:59:38 UTC
Kind of. First, we filtered windows when popping up the menu (i.e. hiding all windows belonging to other applications), then moving through the menu items highlighted the corresponding thumbnail.

I'm sure there are possibilities to highlight windows which feel less busy, but I do see a problem with the way the workspace switcher works - if a window is on a different workspace and completely obscured by other windows, there is nothing in the overview to highlight :)
Comment 4 Allan Day 2011-12-15 12:27:22 UTC
(In reply to comment #3)
> Kind of. First, we filtered windows when popping up the menu (i.e. hiding all
> windows belonging to other applications), then moving through the menu items
> highlighted the corresponding thumbnail.
> 
> I'm sure there are possibilities to highlight windows which feel less busy, but
> I do see a problem with the way the workspace switcher works - if a window is
> on a different workspace and completely obscured by other windows, there is
> nothing in the overview to highlight :)

Yes we need to consider workspaces for this too (ergh). One possibility: add a small amount of opacity to any window or workspace thumbnail that is not associated with the application.
Comment 5 Seif Lotfy 2011-12-18 00:50:35 UTC
Created attachment 203765 [details]
Transparent windows

So based on the patch for bug 666166 I already started working on this issue. Is this the effect you are looking for?
Comment 6 Seif Lotfy 2011-12-18 00:52:38 UTC
Also do we continue highlighting once we right click ?
Comment 7 Allan Day 2011-12-18 17:14:13 UTC
(In reply to comment #5)
> Created an attachment (id=203765) [details]
> Transparent windows
> 
> So based on the patch for bug 666166 I already started working on this issue.
> Is this the effect you are looking for?

Yep! I would use less opacity though.


(In reply to comment #6)
> Also do we continue highlighting once we right click ?

Yes - since the menu blocks a different launcher getting focused.
Comment 8 Allan Day 2012-01-05 10:26:43 UTC
(In reply to comment #5)
> Created an attachment (id=203765) [details]
> Transparent windows
> 
> So based on the patch for bug 666166 I already started working on this issue.
> Is this the effect you are looking for?

Got a patch for this, Seif?
Comment 9 Seif Lotfy 2012-01-05 10:29:42 UTC
I lost my patch will rework it (shouldnt take long)
Comment 10 Stéphane Démurget 2012-11-01 14:14:20 UTC
Just for the information, I've worked on this and it starts to work nicely. I need to see with designers if that matches their expectations and polish my patch a bit before attaching it.
Comment 11 Stéphane Démurget 2012-11-03 16:50:27 UTC
Created attachment 227971 [details] [review]
dash: properly restore item label on popup close

We simply hide the label when the popup is opened instead of relying
on the popup state when the hover state change.

To do this we replace the flag isMenuUp by a 'menu-state-changed' signal
on the AppWellIcon. This simplifies the dash label visibility handling
code that need additional changes for the bug.
Comment 12 Stéphane Démurget 2012-11-03 16:52:16 UTC
Created attachment 227972 [details] [review]
overview: highlight windows on launcher hover

When in the overview, if you move the mouse cursor over one of the
application launchers in the dash, all the unrelated windows are dimmed
both both in the window view and in the workspace view.

It helps to easily understand whether or not there are already opened
windows for this application, and where they are. It can also help in
differentiating the windows in the overview (sometimes the thumbnails
aren't precise enough to easily know which thumbnail belongs to which
application).
Comment 13 Stéphane Démurget 2012-11-03 16:53:29 UTC
Notes:
- I discovered yesterday, I'm the 2nd one to lose my patch!! o_O
  Beware this bug might be doomed! This is a big time loss, but it got better in the end, so it's not _that_ bad.

- I've simplified Dash.getAppFromSource because this also fixes an issue on my box
  (I've got a custom launcher for Firefox/nighly that is not detected in AppSystem and results
   in the awkward situation where no window is dimmed on hover, I suspect this could happen to
   other people)

- I've simplified a bit the logic to only connect once to Main.overview.hiding, which is helpful
  because the code now tracks the hovered item

- I've killed .appIdListToHash, it was unused
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-11-03 17:03:20 UTC
Review of attachment 227972 [details] [review]:

::: js/ui/dash.js
@@ +20,3 @@
 const DASH_ITEM_LABEL_SHOW_TIME = 0.15;
 const DASH_ITEM_LABEL_HIDE_TIME = 0.1;
+const DASH_ITEM_HOVER_TIMEOUT = 200;

Why the change?

@@ +495,2 @@
     _createAppItem: function(app) {
+        let appIcon = new AppDisplay.AppWellIcon(app,

Could you please do this rename in commit beforehand so that the diff is cleaner?

@@ +550,3 @@
+                Mainloop.source_remove(this._setHoverTimeoutId);
+                this._setHoverTimeoutId = 0;
+            }

I don't see the need for the flow change, and different shape of the code hides from the fact that there's a var name change here.

@@ +555,3 @@
+            this._setHoverItem(null);
+
+            if (this._keepHovering)

This is a bad name. Nothing is supposed to "keep hovering"; it's an indication if the user is already hovering. "userAlreadyHovering" might be a bit better.

@@ +564,3 @@
     },
 
+    _setHoverItem: function(item) {

"setHoveredItem", "hoveredItem"

@@ +578,3 @@
+
+        // notifies the focused app change with a small delay to avoid notifying a
+        // temporary 'null' focused app that is of no use when hovering between the dash items

I don't understand this. When you move between the dash items, there's no 'null' item in the middle. The buttons are right on top of each other.

::: js/ui/overview.js
@@ +540,3 @@
 
+    _focusedAppChanged: function(dash, app) {
+        this.focusedApp = app;

Please remove, it's not needed if you use the signal info.

::: js/ui/workspace.js
@@ +1684,3 @@
     },
 
+    _focusedAppChanged: function() {

Use the focusedApp passed in the signal.

::: js/ui/workspaceThumbnail.js
@@ +23,3 @@
+const DIMMED_WINDOW_FADE_IN_TIME = 0.2;
+const DIMMED_WINDOW_FADE_OUT_TIME = 0.15;
+const DIMMED_WINDOW_OPACITY = 40;

Use the same constants as Workspace, please.
Comment 15 Jasper St. Pierre (not reading bugmail) 2012-11-03 17:05:11 UTC
Review of attachment 227971 [details] [review]:

How does this restore the label? I don't see any showHover here.

::: js/ui/dash.js
@@ +510,3 @@
+
+        display.connect('menu-state-changed',
+                        Lang.bind(this, function(icon, opened) {

Put this in a separate function.

@@ +513,3 @@
+                                            if (opened) {
+                                                item.hideLabel();
+                                                this._showLabelTimeoutId = 0;

You need to clear it, no?
Comment 16 Stéphane Démurget 2012-11-03 17:12:46 UTC
A little screencast for designers:

http://zzrough.free.fr/files/public/gnome-shell-657315-Highlight-application-windows-when-hovering-launcher-icon/0001-overview-highlight-windows-on-launcher-hover.webm

Note that I set the dimmed windows opacity very very low in the workspace view
as windows can be layered, contrary to the view selector. It might even make
sense to hide them completely.
Comment 17 Stéphane Démurget 2012-11-03 17:28:14 UTC
Review of attachment 227972 [details] [review]:

::: js/ui/dash.js
@@ +20,3 @@
 const DASH_ITEM_LABEL_SHOW_TIME = 0.15;
 const DASH_ITEM_LABEL_HIDE_TIME = 0.1;
+const DASH_ITEM_HOVER_TIMEOUT = 200;

Clearly an error.

@@ +550,3 @@
+                Mainloop.source_remove(this._setHoverTimeoutId);
+                this._setHoverTimeoutId = 0;
+            }

I'll revert that, it was not "symetric" with the way it's done 3 lines above.

@@ +578,3 @@
+
+        // notifies the focused app change with a small delay to avoid notifying a
+        // temporary 'null' focused app that is of no use when hovering between the dash items

Maybe the explanation needs rewording, this is because we are notified of 'notify::hover signal' once for exiting the first item (this result in _setHoverItem(null)), and set for the second: I do not want the intermediate, null item.

::: js/ui/overview.js
@@ +540,3 @@
 
+    _focusedAppChanged: function(dash, app) {
+        this.focusedApp = app;

I need it in _doAddWindow, this is why I do not use the signal info.

I'll put the full signature though, this is clearer.

::: js/ui/workspaceThumbnail.js
@@ +23,3 @@
+const DIMMED_WINDOW_FADE_IN_TIME = 0.2;
+const DIMMED_WINDOW_FADE_OUT_TIME = 0.15;
+const DIMMED_WINDOW_OPACITY = 40;

This is done on purpose, please see my comment with the video link. If it is the same opacity, you do not see anything in the workspace thumbnail.
Comment 18 Stéphane Démurget 2012-11-03 17:31:33 UTC
Review of attachment 227971 [details] [review]:

::: js/ui/dash.js
@@ +513,3 @@
+                                            if (opened) {
+                                                item.hideLabel();
+                                                this._showLabelTimeoutId = 0;

Nope, the hover event is sent again if the icon gets the focus (seif added a syncHover call in its previous work with labels). Do you want a comment in the code or is there a more clever way of doing it?
Comment 19 Jasper St. Pierre (not reading bugmail) 2012-11-03 17:39:57 UTC
After watching the screencast, it looks nice! I have one thing, though: when clicking on an app that has windows in multiple workspaces, it goes to the workspace with the window on it that was most recently focused. I wonder if it makes sense to only highlight the windows on that workspace in that case, so you have a better idea of where you're going.

(In reply to comment #18)
> Nope, the hover event is sent again if the icon gets the focus (seif added a
> syncHover call in its previous work with labels). Do you want a comment in the
> code or is there a more clever way of doing it?

A comment would be nice.
Comment 20 Stéphane Démurget 2012-11-03 19:26:24 UTC
Created attachment 227979 [details] [review]
dash: properly restore item label on popup close

We simply hide the label when the popup is opened instead of relying
on the popup state when the hover state change.

To do this we replace the flag isMenuUp by a 'menu-state-changed' signal
on the AppWellIcon. This simplifies the dash label visibility handling
code that need additional changes for the bug.
Comment 21 Stéphane Démurget 2012-11-03 19:26:39 UTC
Created attachment 227980 [details] [review]
dash: rename a local variable for clarity
Comment 22 Stéphane Démurget 2012-11-03 19:26:51 UTC
Created attachment 227981 [details] [review]
overview: highlight windows on launcher hover

When in the overview, if you move the mouse cursor over one of the
application launchers in the dash, all the unrelated windows are dimmed
both both in the window view and in the workspace view.

It helps to easily understand whether or not there are already opened
windows for this application, and where they are. It can also help in
differentiating the windows in the overview (sometimes the thumbnails
aren't precise enough to easily know which thumbnail belongs to which
application).
Comment 23 Jasper St. Pierre (not reading bugmail) 2012-11-03 19:30:12 UTC
Review of attachment 227980 [details] [review]:

Some explanation would be nice, but this is fine.
Comment 24 Jasper St. Pierre (not reading bugmail) 2012-11-03 19:32:24 UTC
(In reply to comment #17)
> Maybe the explanation needs rewording, this is because we are notified of
> 'notify::hover signal' once for exiting the first item (this result in
> _setHoverItem(null)), and set for the second: I do not want the intermediate,
> null item.

The leave and enter events will both be dispatched before we tick into the next frame, so the _setHoverItem(null) should have no immediate effect.
Comment 25 Jasper St. Pierre (not reading bugmail) 2012-11-03 19:35:30 UTC
Review of attachment 227979 [details] [review]:

Mostly good.

::: js/ui/dash.js
@@ +525,3 @@
+    _itemMenuStateChanged: function(item, opened) {
+        // the hover event is sent again when the popup is closed so the label will
+        // be shown again if needed (if the item is still hovered)

// When the menu closes, it calls sync_hover, which means
// that the notify::hover handler does everything we need to.

@@ +528,3 @@
+        if (opened) {
+            item.hideLabel();
+            this._showLabelTimeoutId = 0;

Still, I don't understand this. If this is non-zero at any time, we *need* to remove the source, otherwise we leak it, and if it's zero, this is a no-op.
Comment 26 Jasper St. Pierre (not reading bugmail) 2012-11-03 19:46:20 UTC
Review of attachment 227981 [details] [review]:

I think we shouldn't use the term "focused app" -- seems like it might be confused with the app that contains the window that's currently focused. "Hovered item" or "hovered app" is fine with me.

::: js/ui/dash.js
@@ +25,3 @@
 function getAppFromSource(source) {
     if (source instanceof AppDisplay.AppWellIcon) {
+        return source.app;

Separate patch, please.

(Although good eye!)

@@ +573,3 @@
+            return;
+
+        // changes the hover item

Useless comment.

@@ +576,3 @@
+        this._hoveredItem = item;
+
+        // gets the focused app

Useless comment.

::: js/ui/workspace.js
@@ +200,3 @@
+        let time = dimmed ? DIMMED_WINDOW_FADE_IN_TIME : DIMMED_WINDOW_FADE_OUT_TIME;
+
+        if (this.actor.opacity != opacity)

This is wrong.

@@ +1016,3 @@
         }
 
+        // Dim the windows that are not part of the focused application

Useless comment.

@@ +1367,3 @@
+
+        // this window might be part of an application not having the focus so we update its
+        // dimmed status without animation

Remove this comment. It's just confused me.

@@ +1691,3 @@
+
+    _updateCloneDimmed: function(clone, withAnimation) {
+        let focusedApp = Main.overview.focusedApp;

As I said in the last review, please use the app sent in the signal.

@@ +1695,3 @@
+
+        let dimmed = (focusedApp != null && app != focusedApp);
+

Whitespace is a bit silly.

::: js/ui/workspaceThumbnail.js
@@ +22,3 @@
 
+const DIMMED_WINDOW_FADE_IN_TIME = 0.2;
+const DIMMED_WINDOW_FADE_OUT_TIME = 0.15;

At least use the FADE_IN / FADE_OUT constants from Workspace?

@@ +23,3 @@
+const DIMMED_WINDOW_FADE_IN_TIME = 0.2;
+const DIMMED_WINDOW_FADE_OUT_TIME = 0.15;
+const DIMMED_WINDOW_OPACITY = 40;

Put a comment explaining why you aren't using the DIMMED_WINDOW_OPACITY from Workspace.
Comment 27 Stéphane Démurget 2012-11-03 21:21:30 UTC
Created attachment 227985 [details] [review]
dash: properly restore item label on popup close

We simply hide the label when the popup is opened instead of relying
on the popup state when the hover state change.

To do this we replace the flag isMenuUp by a 'menu-state-changed' signal
on the AppWellIcon. This simplifies the dash label visibility handling
code that need additional changes for the bug.
Comment 28 Stéphane Démurget 2012-11-03 21:21:43 UTC
Created attachment 227986 [details] [review]
dash: simplify app retrieval from AppWellIcon

This has also the benefit of getting the application even if it can not
be retrieved through AppSystem, which can happen if the runtime WMClass
does not match the one of the desktop file.

This especially looked wrong with the following commits related to the
bug.
Comment 29 Stéphane Démurget 2012-11-03 21:22:01 UTC
Created attachment 227987 [details] [review]
overview: highlight windows on launcher hover

When in the overview, if you move the mouse cursor over one of the
application launchers in the dash, all the unrelated windows are dimmed
both both in the window view and in the workspace view.

It helps to easily understand whether or not there are already opened
windows for this application, and where they are. It can also help in
differentiating the windows in the overview (sometimes the thumbnails
aren't precise enough to easily know which thumbnail belongs to which
application).
Comment 30 Stéphane Démurget 2012-11-04 10:19:37 UTC
(In reply to comment #19)
> After watching the screencast, it looks nice! I have one thing, though: when
> clicking on an app that has windows in multiple workspaces, it goes to the
> workspace with the window on it that was most recently focused. I wonder if it
> makes sense to only highlight the windows on that workspace in that case, so
> you have a better idea of where you're going.

This is up to designers, but indeed we could associate the default action
(left button with mouse) of an active application to the window that would get
the focus on a click (this is what you mean I think).

I think only highlighting one window would defeat the point of the patch though, which is to show all the opened windows of an app. Even if it means a bit more clutter, I'd highlight this very window in a different way somehow?
Comment 31 Allan Day 2012-11-05 10:37:41 UTC
Created attachment 228091 [details]
screenshot
Comment 32 Allan Day 2012-11-05 10:38:13 UTC
Created attachment 228092 [details]
screenshot
Comment 33 Allan Day 2012-11-05 10:42:00 UTC
Hey Stéphane! This looks good to me. The only change I'd make is not to dim any windows while hovering over a launcher that isn't running. I'd like to get a second opinion before going forward with this though. Jon & Jimmac - what do you think?
Comment 34 Stéphane Démurget 2012-11-05 13:02:14 UTC
Allan: I indeed thought about that, and played with it, but I do not know, it didn't feel "coherent", but that's just my opinion.

When the user looks for an app or launcher for something and move the cursor between launchers, this will bring attention by "blinking smoothly".

The argument could also reversed as it dims windows for no purpose when you are hovering on non-running applications indeed.
Comment 35 Jakub Steiner 2012-11-06 14:37:47 UTC
Definitely wouldn't shade windows when hovering over apps that dont have any windows visible. The purpose I see is to aid locating the app windows.
Comment 36 Julien Olivier 2012-11-06 15:01:14 UTC
(In reply to comment #33)
> Hey Stéphane! This looks good to me. The only change I'd make is not to dim any
> windows while hovering over a launcher that isn't running. I'd like to get a
> second opinion before going forward with this though. Jon & Jimmac - what do
> you think?

Hi,

I guess you've thought about it too, but why don't you draw a colored background behind the windows matching the application launcher (and use the same color to highlight the launcher itself too) instead of dimming all the other ones? This way, if a launcher has no window, all the windows appear as normal, and if he does, its windows are highlighted by the colored background.

Basically, my idea is just to highlight the matching windows instead of dimming the other windows...
Comment 37 Florian Müllner 2012-11-06 15:40:39 UTC
(In reply to comment #36)
> Basically, my idea is just to highlight the matching windows instead of dimming
> the other windows...

The highlight will be invisible for windows that are not on the active workspace and completely obscured by other windows - see comments #3 and #4..
Comment 38 Julien Olivier 2012-11-06 15:48:34 UTC
> The highlight will be invisible for windows that are not on the active
> workspace and completely obscured by other windows - see comments #3 and #4..

Oh, right, I forgot about it... Maybe the obscured windows could just pop in front in the workspace switcher?
Comment 39 Stéphane Démurget 2012-11-06 21:17:12 UTC
Created attachment 228310 [details] [review]
dash: properly restore item label on popup close

We simply hide the label when the popup is opened instead of relying
on the popup state when the hover state change.

To do this we replace the flag isMenuUp by a 'menu-state-changed' signal
on the AppWellIcon. This simplifies the dash label visibility handling
code that need additional changes for the bug.
Comment 40 Stéphane Démurget 2012-11-06 21:17:20 UTC
Created attachment 228311 [details] [review]
dash: rename a local variable for clarity

createAppIcon creates an appIcon, not a 'display', which made things
unnecessary more difficult to understand.
Comment 41 Stéphane Démurget 2012-11-06 21:17:27 UTC
Created attachment 228312 [details] [review]
dash: simplify app retrieval from AppWellIcon

This has also the benefit of getting the application even if it can not
be retrieved through AppSystem, which can happen if the runtime WMClass
does not match the one of the desktop file.

This especially looked wrong with the following commits related to the
bug.
Comment 42 Stéphane Démurget 2012-11-06 21:17:34 UTC
Created attachment 228313 [details] [review]
overview: highlight windows on launcher hover

When in the overview, if you move the mouse cursor over one of the
application launchers in the dash, all the unrelated windows are dimmed
both both in the window view and in the workspace view.

It helps to easily understand whether or not there are already opened
windows for this application, and where they are. It can also help in
differentiating the windows in the overview (sometimes the thumbnails
aren't precise enough to easily know which thumbnail belongs to which
application).
Comment 43 Stéphane Démurget 2012-11-06 21:25:51 UTC
This patche series:
- is rebased against master
- contains a comment for the variable rename
- fixes the behavior to be only for running application
- adds secondary action support (Alt) to not consider the running state (suggested by jimmac on IRC, this is what I initially implemented, "that makes it a useful aid for heavy taskers while avoiding complexity for the masses")

Jasper, I have kept the "hovered-app-changed" signal but I do not like it, now there are two different reaction, can I rename it to something more "precise" (I do not know how to say that: app-centric, not generic) like "reveal-app-windows" and instead primaryAction use onlyForRunningApp (ouch?). That would also avoid to send spurious changes from the dash if the event does not mean a real change.
Comment 44 Jasper St. Pierre (not reading bugmail) 2012-11-06 21:41:58 UTC
Review of attachment 228310 [details] [review]:

Looks good.
Comment 45 Jasper St. Pierre (not reading bugmail) 2012-11-06 21:51:27 UTC
Review of attachment 228311 [details] [review]:

Bit of strange grammar in the commit message; I think you meant "unnecessarily more difficult". Code's fine otherwise.
Comment 46 Jasper St. Pierre (not reading bugmail) 2012-11-06 21:52:00 UTC
Review of attachment 228312 [details] [review]:

Yes.
Comment 47 Jasper St. Pierre (not reading bugmail) 2012-11-06 22:05:45 UTC
Review of attachment 228313 [details] [review]:

::: js/ui/dash.js
@@ +491,3 @@
     },
 
+    _hookUpItem: function(item) {

hookUpItemHover

@@ +516,2 @@
         let item = new DashItemContainer();
+        item.appIcon = appIcon;

We try not to add properties like this to random objects that we don't control. There has to be code elsewhere that does things like this, maybe using _delegate -- see if you can find and adapt it.

@@ +590,3 @@
+
+        // The leave and enter events will both be dispatched before we tick into the next
+        // frame, so the _setHoverItem(null) should have no immediate effect.

This was meant as a rationale for you to take out the HOVERED_APP_NOTIFY, not meant as a comment for you to put in.

@@ +602,3 @@
+    },
+
+    _onCapturedEvent: function(actor, event) {

Not a fan of this at all. Let's leave the ALT part out for now.
Comment 48 Allan Day 2012-11-06 22:10:33 UTC
I'd like Jon to review this before we land it.
Comment 49 Jasper St. Pierre (not reading bugmail) 2012-11-06 22:17:49 UTC
Note that the first three patches are just cleanups and can (and should) be pushed now without any UI changes or anything like that.
Comment 50 Stéphane Démurget 2012-11-06 23:41:15 UTC
Comment on attachment 228310 [details] [review]
dash: properly restore item label on popup close

pushed as ee2f12fe819cf9b5f9648b92d86346f95802da83
Comment 51 Stéphane Démurget 2012-11-06 23:41:41 UTC
Comment on attachment 228311 [details] [review]
dash: rename a local variable for clarity

pushed as df0151d338248ae7cd38c41cda7a0438bdb2c880
Comment 52 Stéphane Démurget 2012-11-06 23:42:07 UTC
Comment on attachment 228312 [details] [review]
dash: simplify app retrieval from AppWellIcon

pushed as 95abdeb919083809c30528c32e844d10b4bdfda9
Comment 53 Stéphane Démurget 2012-11-07 13:29:18 UTC
Review of attachment 228313 [details] [review]:

I've reworked my patch, but I'm a bit puzzled re: the appIcon property.

::: js/ui/dash.js
@@ +516,2 @@
         let item = new DashItemContainer();
+        item.appIcon = appIcon;

Aren't the dash responsible of the objects it creates? This is not a random object, we are talking about the DashItemContainer of the same file, both can not be tightly coupled?

I can pass it through the constructor if you prefer, and/or subclass the DashItemContainer as  AppDisplayIcon or something if it makes more sense.

For me a delegate is used to forward a contract. I've checked how it's used in different locations, but I fail to see the point here?
Comment 54 William Jon McCann 2012-11-08 18:40:35 UTC
I'm sorry I missed this bug until recently. Thanks for experimenting with this!

We used to have something like this but not done as well. It was removed for a few reasons. Mostly that it was disruptive when you were moving over the dash. Certainly worth taking another crack at it.

I have a number of concerns though.

The first is that I'm not sure it is really solving the core problem. I think there are indeed cases where it can be hard to differentiate window previews. For that, I think we will probably have to address the window previews themselves. For me I find it really awkward to have to think about going to another place in the overview to help me filter out windows. It still could be a fine optional or advanced user feature if it didn't interfere with anything else. But I'm not sure that is the case.

I find it really distracting to see windows flashing as I move the mouse across the dash. I want my focus to be where I'm pointing and having things moving in the periphery is problematic.

Relying on hover as a solution is problematic for touch devices. If we really do have a differentiation problem (I think we do) then a solution should be found for touch devices as well.

When I have multiple windows open for an app and I hover over the launcher I see all the windows highlighted. However, when I click the launcher I only get one window from that app focused and there is no indication of which window that will be.

In the case where I have only one window open per app I think it is a bit odd to see the filtering done in a way that separates where I am acting from where I am looking. We could probably do something just as good by having an app icon appear over the window preview on hover. (If we want to rely on hover)

Identifying the windows for an app is presumably most useful for actually getting to a window - not an app. In that case I can't really just click on the app icon - I need to move over to the window picker to select a window. However, I don't like that once I have used the filtering to identify the windows for an app, the filtering goes away when I move over to click on the desired window.

I find differentiation of windows within an app to be just as problematic as differentiation of windows from separate apps. I think many user complaints about the difficulty of finding terminal windows supports this. It doesn't seem to me that the app filtering alone does anything to address this. (The historical version of this feature also magnified the windows IIRC).

With respect to the filtering itself I think there are a few issues.

The labels are not filtered along with the window previews. I think they should be.

There is no filtering done when moving over the window list items in the context menu of dash launchers. This is actually where I think it could be most interesting to do filtering.


How about something like this...
 * also filter out window captions
 * only do the filtering when the context menu for the app launcher is up
 * do something smart when hovering over New window in the context menu
 * identify the window that will be focused as I move over the window list in the context menu
 * identify the window that will be focused when the launcher is clicked

I think having the filtering be related to the window list in the context menu makes some sense and it can help the user make a decision on whether to use New Window or not by seeing what windows are currently open.

And continue to seek improvements to the window selector itself.
Comment 55 Florian Müllner 2012-11-08 18:56:40 UTC
(In reply to comment #54)
> [...]
> I find differentiation of windows within an app to be just as problematic as
> differentiation of windows from separate apps. I think many user complaints
> about the difficulty of finding terminal windows supports this. It doesn't seem
> to me that the app filtering alone does anything to address this. (The
> historical version of this feature also magnified the windows IIRC).

In a way, yes. Technically we hid all windows from other applications, so the remaining window previews could take up more space.


> How about something like this...
>  [...]
>  * only do the filtering when the context menu for the app launcher is up
>  [...]
>  * identify the window that will be focused as I move over the window list in
> the context menu

That's actually very close to what we used to do:
When opening the context menu, all windows from other applications were filtered out and the remaining previews were slightly dimmed. When moving through the window list in the menu, the corresponding window preview was highlighted by making it fully opaque.

The main issue was that when we changed the overview to only fully display the active workspace, the implementation was quite confusing when windows were spread over several workspaces, in particular with none on the active one.
Comment 56 Stéphane Démurget 2012-11-08 19:33:59 UTC
(In reply to comment #54)
> There is no filtering done when moving over the window list items in the
> context menu of dash launchers. This is actually where I think it could be most
> interesting to do filtering.

I learned something here... I did not know there was a window selector there. From 3.0 I only used the context menu once for "New Window" menu and a couple of times later to setup my favorites. I never opened that context menu again. Now I see there is a window list in there, I wonder who would open up a context menu just to see small targets, textual representation of windows when you've got a selector with big windows?

I really expect the whole shell to work without needing context menus... 

> I think having the filtering be related to the window list in the context menu
> makes some sense and it can help the user make a decision on whether to use New
> Window or not by seeing what windows are currently open.

Unfortunately, I think nobody will ever open this context menu except on touch devices. We really need to have something working for all targets. :-/

If it is only to find out the windows of an application, I suppose grouping the windows by app a-la-mission control would proove to be easier on desktop and on touch.
Comment 57 Carlos Soriano 2012-11-18 16:22:05 UTC
I did a extension for make more usable the windows picker some time before.
You can get ideas from here. I wanted to make also something like mission-control to resolve the difficult windows picking of the same app between all the windows apps.

Video of the extension:
http://www.youtube.com/watch?v=QWWoxJXMzJ4

The former idea was to group all windows a la mission control, and then, if you hover that group in the windows picker, the group will expand to show the major part of those app-windows, lowering opacity of the other groups and moving them(in a logical form to not make crazy the user) to make space to the hovered-app-windows. Also, apply the behaviour saw in the extension, if you hover some icon-app, hiding all the other windows.

I don't know which is the best idea for this....but sure the current behaviour is difficult to use.
Comment 58 Allan Day 2012-11-20 10:10:51 UTC
(In reply to comment #54)
> How about something like this...
>  * also filter out window captions
>  * only do the filtering when the context menu for the app launcher is up
>  * do something smart when hovering over New window in the context menu
>  * identify the window that will be focused as I move over the window list in
> the context menu
>  * identify the window that will be focused when the launcher is clicked

I like all of this. The only issue I can see is relying on the context menu, since it may overlap the thumbnails you are interested in. I get the intention behind that though - it means that we won't be relying on hover.

We need to find an answer to that design question...
Comment 59 Florian Müllner 2017-12-07 23:52:44 UTC
*** Bug 791364 has been marked as a duplicate of this bug. ***
Comment 60 GNOME Infrastructure Team 2021-07-05 14:04:57 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of  gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/

Thank you for your understanding and your help.