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 694914 - classic mode: add a workspace indicator
classic mode: add a workspace indicator
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
classic
Depends on:
Blocks:
 
 
Reported: 2013-03-01 04:03 UTC by Matthias Clasen
Modified: 2013-04-16 20:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window-list: add a workspace switcher (6.53 KB, patch)
2013-03-04 17:55 UTC, Giovanni Campagna
none Details | Review
Screenshot with patch applied (246.17 KB, image/png)
2013-03-06 21:23 UTC, Florian Müllner
  Details
WindowList: add a workspace switching menu (6.14 KB, patch)
2013-03-20 15:57 UTC, Giovanni Campagna
reviewed Details | Review
window-list: add a workspace switcher (6.78 KB, patch)
2013-03-20 16:17 UTC, Giovanni Campagna
rejected Details | Review
screenshot of the menu (28.10 KB, image/png)
2013-04-03 19:03 UTC, Matthias Clasen
  Details
screenshot of the switcher (11.82 KB, image/png)
2013-04-03 19:11 UTC, Matthias Clasen
  Details
Screenshot of the menu in normal session (868 bytes, image/png)
2013-04-05 14:31 UTC, Florian Müllner
  Details
WindowList: add a workspace switching menu (5.92 KB, patch)
2013-04-10 16:30 UTC, Giovanni Campagna
reviewed Details | Review
WindowList: add a workspace switching menu (5.70 KB, patch)
2013-04-16 19:18 UTC, Giovanni Campagna
committed Details | Review

Description Matthias Clasen 2013-03-01 04:03:27 UTC
I can see at least two ways of doing this:

- we could use a menu like the workspace indicator extension, spruce it up a bit by adding thumbnails, and put it in the bottom panel

- or bit the bullet and just put a gnome2-style horizontal pager in there (need to make sure both horizontal and vertical keybindings work, then)
Comment 1 Giovanni Campagna 2013-03-02 18:23:45 UTC
If we want the horizontal pager, Frippery Bottom Panel had it. For keybindings, it's easy to set ctrl-alt-left/right to be workspace up/down instead of left/right, without modifying the vertical layout.

The problem is how to make it optional, because currently we can't have two extensions that interact with each other in a sane way. But if we just want the window list to always have a panel, I can just rip off the existing code.
Comment 2 Florian Müllner 2013-03-02 18:29:31 UTC
(In reply to comment #1)
> The problem is how to make it optional, because currently we can't have two
> extensions that interact with each other in a sane way.

If we want to make it optional, we could add a 'show-pager' key to the window-list schema :-)
Comment 3 Giovanni Campagna 2013-03-02 18:33:49 UTC
Oh sorry, I didn't know we had a configuration system in gnome-shell :P

Anyway, I guess it is possible to extend the window list then.
Is it a requirement to have window shapes/thumbnails? Or just the list with the currently selected one?
Comment 4 Florian Müllner 2013-03-02 18:43:55 UTC
Well, I mentioned this briefly to the designers at fosdem, and their initial thoughts were to:

  - use a GNOME2-like horizontal pager
  - make the workspace layout horizontal
  - either remove the workspace switcher from the overview,
    or disable the overview altogether

For the sake of sanity, I suggest not doing that :-)


(In reply to comment #3)
> Anyway, I guess it is possible to extend the window list then.
> Is it a requirement to have window shapes/thumbnails? Or just the list with the
> currently selected one?

I'd say it depends on which option we choose - in a horizontal pager, the thumbnail size would be far too small to be usable in my opinion, so I'd just use rectangles in that case (possibly showing windows as rectangles as well á la GNOME2, though I wouldn't bother to make those draggable).

Thanks for tackling this btw.
Comment 5 Matthias Clasen 2013-03-03 01:23:25 UTC
My opinions:

- we  should  not disable the overview entirely. That would make search and expose unavailable and render the search panel useless.

- not having window dragging is fine with me
Comment 6 Matthias Clasen 2013-03-03 01:25:07 UTC
- i don't think making it optional is  very important
Comment 7 Matthias Clasen 2013-03-03 01:25:47 UTC
My opinions:

- we  should  not disable the overview entirely. That would make search and expose unavailable and render the search panel useless.

- not having window dragging is fine with me
Comment 8 Giovanni Campagna 2013-03-04 17:55:35 UTC
Created attachment 238025 [details] [review]
window-list: add a workspace switcher

This is the most basic version of a workspace switcher, taken from
Frippery Bottom Panel and adapted. It handles clicks and scrolls,
and does not show window thumbnails or shapes.
Note that, differently from the frippery version, it won't change
the workspace layout, and actually assume a linear vertical layout,
which is then shown horizontally. This is to keep compatibility
with the overview, which uses a vertical layout.
Comment 9 Florian Müllner 2013-03-06 21:23:41 UTC
Created attachment 238250 [details]
Screenshot with patch applied
Comment 10 Matthias Clasen 2013-03-07 17:37:44 UTC
We should probably make it handle dnd hover
Comment 11 Florian Müllner 2013-03-07 17:46:39 UTC
(In reply to comment #10)
> We should probably make it handle dnd hover

Unfortunately the most useful case - window dnd - is hard ...
Comment 12 Allan Day 2013-03-07 19:17:31 UTC
Apologies for the slow response here. I largely agree with Matthias in the original bug report. So, two possible approaches:

1. Use a GNOME 2 style horizontal switcher in the panel. This would require changing the orientation of the workspaces to horizontal, hiding the workspace switcher in the overview, and changing the keyboard shortcuts.

2. Use a menu in the panel: this could simply read "1/4" (for example), and each item in the menu could be a number.

+-----+
| * 1 |
|   2 |
|   3 |
|   4 |
+-- --+
   V
 1/4 v
Comment 13 Florian Müllner 2013-03-07 19:30:34 UTC
(In reply to comment #12)
> 1. Use a GNOME 2 style horizontal switcher in the panel. This would require
> changing the orientation of the workspaces to horizontal, hiding the workspace
> switcher in the overview, and changing the keyboard shortcuts.

If all this is required (e.g. we are not OK with the inconsistency between switcher layout and workspace layout), then this is not an option.
Comment 14 Florian Müllner 2013-03-07 20:23:23 UTC
(In reply to comment #13)
> If all this is required (e.g. we are not OK with the inconsistency between
> switcher layout and workspace layout), then this is not an option.

OK, I was probably a bit too quick here - if we implement both options and pick the correct one based on the workspace layout, we can have another extension to lay out workspaces horizontally (it will still be a bit messy, but hopefully manageable)
Comment 15 Matthias Clasen 2013-03-08 11:46:21 UTC
Allan, do you think horizontal-in-panel / vertical-in-overview is unacceptable ?
Comment 16 Florian Müllner 2013-03-08 12:10:08 UTC
(In reply to comment #15)
> Allan, do you think horizontal-in-panel / vertical-in-overview is unacceptable
> ?

I can't respond for Allan, but he's the third designer who suggests swapping the workspace layout ...
Comment 17 Allan Day 2013-03-08 12:18:43 UTC
(In reply to comment #15)
> Allan, do you think horizontal-in-panel / vertical-in-overview is unacceptable
> ?

It would be rather odd, wouldn't it? Clicking on the horizontal switcher would result in a vertical transition...
Comment 18 Matthias Clasen 2013-03-14 12:08:14 UTC
I've tried Giovannis patch now. While I personally don't mind the vertical movement triggered by a horizontal list of buttons, the buttons themselves don't really convince me.

 * The text displayed is just the workspace number, and for the active workspace, it is -1-. Doesn't look very professional, I'd try bold for the active workspace

 * The outline of the buttons seems to be cut off at the bottom here

 * When clicking around without moving any windows, I see the number of buttons change. I have 1, 2, 3. I click on 3, go to that empty workspace, then click on 2, and the switcher changes to showing just 1 and 2, with neither marked as active. Clicking around further brings the number back to 3. This was with dynamic workspaces

Allan, can you try the patch and give me an up-or-down vote on whether the horizontal layout with vertical workspaces is an absolute no-go for you ? I would hate to turn off the actual workspace switcher in the overview. It is much more useful than this thing, and so far we've managed to avoid mutilating the overview for classic mode...
Comment 19 Giovanni Campagna 2013-03-20 15:57:53 UTC
Created attachment 239366 [details] [review]
WindowList: add a workspace switching menu

Mostly a copy of the existing workspace switcher extension, with
minor tweaks.

For the sake of the discussion, this is the workspace menu Allan proposed in comment 12.
Comment 20 Giovanni Campagna 2013-03-20 16:17:21 UTC
Created attachment 239370 [details] [review]
window-list: add a workspace switcher

This is the most basic version of a workspace switcher, taken from
Frippery Bottom Panel and adapted. It handles clicks and scrolls,
and does not show window thumbnails or shapes.
Note that, differently from the frippery version, it won't change
the workspace layout, and actually assume a linear vertical layout,
which is then shown horizontally. This is to keep compatibility
with the overview, which uses a vertical layout.

And this is the fixed version of the horizontal switcher.
Comment 21 Matthias Clasen 2013-04-03 19:03:59 UTC
Created attachment 240535 [details]
screenshot of the menu
Comment 22 Matthias Clasen 2013-04-03 19:11:31 UTC
Created attachment 240536 [details]
screenshot of the switcher
Comment 23 Matthias Clasen 2013-04-03 19:14:11 UTC
The switcher works ok now. But given that it doesn't contain more information than the menu, I feel it uses too much space. Lets go with the menu, I'd say.
Comment 24 Florian Müllner 2013-04-05 14:30:42 UTC
Comment on attachment 239370 [details] [review]
window-list: add a workspace switcher

Allan was OK with going with the menu, so let's do that ...
Comment 25 Florian Müllner 2013-04-05 14:31:34 UTC
Created attachment 240750 [details]
Screenshot of the menu in normal session
Comment 26 Florian Müllner 2013-04-05 14:59:53 UTC
Review of attachment 239366 [details] [review]:

::: extensions/window-list/extension.js
@@ +25,3 @@
 
+const WORKSPACE_SCHEMA = 'org.gnome.desktop.wm.preferences';
+const WORKSPACE_KEY = 'workspace-names';

I don't share Jasper's disgust of those "defines", but it's a bit odd to use a string constant for this key and not for 'grouping-mode'

@@ +371,3 @@
+
+	this.workspacesItems = [];
+	this._workspaceSection = new PopupMenu.PopupMenuSection();

There's nothing else in the menu, so why not add/remove items directly?

@@ +413,3 @@
+    },
+
+    _createWorkspacesSection : function() {

The function isn't really creating anything, it's updating the section's content - so _updateWorkspaceSection / _updateWorkspaces makes more sense to me (also style nit: space before the colon)

@@ +423,3 @@
+            item.workspaceId = i;
+
+	    item.connect('activate', Lang.bind(this, function(actor, event) {

Using "actor" for anything which is not derived from ClutterActor is just confusing. I'd either reuse 'item' or omit the function parameters altogether

@@ +437,3 @@
+    },
+
+    _activate : function (index) {

Space before colon

@@ +444,3 @@
+    },
+
+    _onScrollEvent : function(actor, event) {

Dto.

@@ +455,3 @@
+	}
+
+	let newIndex = global.screen.get_active_workspace().index() + diff;

That's "this._currentWorkspace + diff", no?
(Also, I would do the bound checking here ("if (direction == Clutter.ScrollDirection.DOWN && index > 0)") instead of inside _activate(), but that's just a personal preference)

@@ +492,3 @@
+        box.add(this._workspaceIndicator.container);
+
+        this.menuManager = new PopupMenu.PopupMenuManager(this);

Why is this public?

::: extensions/window-list/stylesheet.css
@@ +68,3 @@
+
+.window-list-workspace-indicator {
+  background-color: rgba(200, 200, 200, .5);

That looks rather in-your-face to me, I'd go with something more subtle (or no background at all). I'd say let's make Jakub take a look ...
Comment 27 Giovanni Campagna 2013-04-10 15:23:28 UTC
Closed by mistake, I assume.
Comment 28 Giovanni Campagna 2013-04-10 16:30:20 UTC
Created attachment 241189 [details] [review]
WindowList: add a workspace switching menu

Import a copy of the workspace indicator extension, to have it
in the bottom panel.
Comment 29 Matthias Clasen 2013-04-15 12:38:41 UTC
I would really like to have this in 3.8.1 - any chance ?
Comment 30 Florian Müllner 2013-04-15 12:40:11 UTC
Yes, I'll do another review in a bit ...
Comment 31 Florian Müllner 2013-04-16 16:46:42 UTC
Review of attachment 241189 [details] [review]:

Better, but still some (old and new) nits left ...

::: extensions/window-list/extension.js
@@ -25,3 @@
 };
 
-

Unrelated whitespace change *shrug*

@@ +438,3 @@
+
+	    item.connect('activate', Lang.bind(this, function(actor, event) {
+		this._activate(actor.workspaceId);

The previous comment about (ab)using "actor" for something not a ClutterActor still applies.

@@ +451,3 @@
+    },
+
+    _activate: function (index) {

Style nit: space before parenthesis

::: extensions/window-list/stylesheet.css
@@ +69,3 @@
+.window-list-workspace-indicator {
+  background-color: rgba(200, 200, 200, .3);
+  border: 1px solid #cccccc;

I'm still not sold on the styling - it looks fine in classic mode, but fairly strong in a normal session ...
Comment 32 Giovanni Campagna 2013-04-16 19:18:08 UTC
Created attachment 241678 [details] [review]
WindowList: add a workspace switching menu

Import a copy of the workspace indicator extension, to have it
in the bottom panel.

I did not change the styling this time, because, well, I'm not
a designer, so I really can't tell - it looks decent to me...
Comment 33 Florian Müllner 2013-04-16 19:44:18 UTC
Review of attachment 241678 [details] [review]:

I guess the whole non-classic style could use some design love, so let's not block in that ...

Last nit though (no re-review needed): you are using a wild mix of spaces/tab indentation; for consistency, please replace the tabs before pushing, thanks!
Comment 34 Giovanni Campagna 2013-04-16 20:09:14 UTC
Attachment 241678 [details] pushed as c1bc688 - WindowList: add a workspace switching menu