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 642189 - Remove window filtering for dash item right click menu
Remove window filtering for dash item right click menu
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-02-12 18:49 UTC by Owen Taylor
Modified: 2011-02-14 22:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
AppWellMenu: Remove window filtering (10.09 KB, patch)
2011-02-12 19:09 UTC, drago01
needs-work Details | Review
AppWellMenu: Remove window filtering (10.87 KB, patch)
2011-02-13 16:43 UTC, drago01
needs-work Details | Review
AppWellMenu: Remove window filtering (16.03 KB, patch)
2011-02-13 21:27 UTC, drago01
needs-work Details | Review
AppWellMenu: Remove window filtering (16.03 KB, patch)
2011-02-13 22:12 UTC, drago01
none Details | Review
AppWellMenu: Remove window filtering (16.05 KB, patch)
2011-02-14 22:01 UTC, drago01
committed Details | Review

Description Owen Taylor 2011-02-12 18:49:29 UTC
From https://bugzilla.gnome.org/show_bug.cgi?id=639567#c6

the whole "filter windows on right click on a dash item" thing isn't working:

 * Having everything change when you click on the menu is really distracting
and confusing
 * When you go over the menu, having things prelight in the menu, and then also
change elsewhere on the screen means you don't know where to look
 * The highlight behavior with a black box over the workspace area is just
weird - it was a hack that sort of worked with how we drew the workspace at the
time.
 * In a multiple workspaces scenario, showing only the app windows from this workspace is confusing, but pulling them all together without any context is also
weird

So we want to just remove it. I'll file a separate bug for that rather than
repurposing this one.

I think it's best to be fairly aggressive about removing code - there is so much going on in the workspace window positioning code that code portions that aren't being used will rot very quickly, and I think it's unlikely we'd want to bring this behavior back in anything like its current form.
Comment 1 drago01 2011-02-12 19:09:46 UTC
Created attachment 180732 [details] [review]
AppWellMenu: Remove window filtering

Don't filter and highlight windows when opening the menu
as this turned out to be distracting and confusing.

---

Well so lets kill it ...
Comment 2 Owen Taylor 2011-02-13 14:40:23 UTC
Review of attachment 180732 [details] [review]:

Looks good as far as it goes, but I think you should strip out Workspace.setShowOnlyWindows and the implementation of that as well.

::: js/ui/appDisplay.js
@@ -457,3 @@
 
-    highlightWindow: function(metaWindow) {
-        if (this._didActivateWindow)

I don't see any reason to keep didActivateWindow around - it's about not undoing the filtering when zooming into a desktop.
Comment 3 drago01 2011-02-13 16:43:48 UTC
Created attachment 180778 [details] [review]
AppWellMenu: Remove window filtering

Don't filter and highlight windows when opening the menu
as this turned out to be distracting and confusing.
Comment 4 Owen Taylor 2011-02-13 20:22:44 UTC
Review of attachment 180778 [details] [review]:

::: js/ui/appDisplay.js
@@ -466,3 @@
     activateWindow: function(metaWindow) {
         if (metaWindow) {
             this._didActivateWindow = true;

This needed to be removed too (so just search through the file for didActivateWindow)

::: js/ui/workspace.js
@@ -614,3 @@
     },
 
-    setShowOnlyWindows: function(showOnlyWindows, reposition) {

There's a lot more to the implementation than this.
Comment 5 drago01 2011-02-13 21:27:01 UTC
Created attachment 180789 [details] [review]
AppWellMenu: Remove window filtering

Don't filter and highlight windows when opening the menu
as this turned out to be distracting and confusing.

---

Should be complete this time unless I missed anything again.
Comment 6 Owen Taylor 2011-02-13 22:05:35 UTC
Review of attachment 180789 [details] [review]:

Looks very close

::: js/ui/workspace.js
@@ +850,2 @@
         if (this._reservedSlot)
+            clones.push(this._reservedSlot);

you didn't copy this._windows, so now you modified it.
Comment 7 drago01 2011-02-13 22:12:38 UTC
Created attachment 180790 [details] [review]
AppWellMenu: Remove window filtering

Don't filter and highlight windows when opening the menu
as this turned out to be distracting and confusing.
Comment 8 drago01 2011-02-14 22:01:35 UTC
Created attachment 180858 [details] [review]
AppWellMenu: Remove window filtering

Don't filter and highlight windows when opening the menu
as this turned out to be distracting and confusing.

---

*) Don't sort this._windows, but a copy of it
Comment 9 Owen Taylor 2011-02-14 22:04:14 UTC
Review of attachment 180858 [details] [review]:

Both fixes to use .slice() look good
Comment 10 drago01 2011-02-14 22:07:36 UTC
Attachment 180858 [details] pushed as c2ae95f - AppWellMenu: Remove window filtering