Created attachment 152089 [details] [review]
Fix transition to zoomed window in single view

Fix messed up transition from overview when activating a zoomed in
window.

Created attachment 152090 [details] [review]
Fix slightly misplaced overlays in single view

Correct the positioning of window captions and close buttons for
non-active workspaces in single view.

Created attachment 152091 [details] [review]
[Overview] CSS tweaks to match mockups better

- add some spacing between buttons
- move controls closer to the workspaces view (we'll need that space
  for the message tray)
- fix the look of the scrollbar background
- some general CSS cleanup

Created attachment 152092 [details] [review]
Make inactive workspace buttons insensitive

Update sensitivity instead of hiding buttons - hiding the
add workspace button looks especially weird.

Created attachment 152243 [details] [review]
Reduce the gap between workspaces in linear view

Spacing between buttons and workspaces are nice changes.  Making it look more like the mockups is almost always good :)

Not sure if it is a result of this patch or it was there before but the grid and single buttons are different sizes or at least the single one seems to not be aligned to the top of the grid one.  If this was in the mockup it was a mistake.

Also, the prelighting on the scrollbar seems to only happen when leaving it or something.

(In reply to comment #6)
> Not sure if it is a result of this patch or it was there before but the grid
> and single buttons are different sizes or at least the single one seems to not
> be aligned to the top of the grid one.  If this was in the mockup it was a
> mistake.

Not a result of the patch, but I'll attach an update.


> Also, the prelighting on the scrollbar seems to only happen when leaving it or
> something.

Yup, that's a scrollbar bug - it is much more obvious with the workspaces scrollbar, but if looking closely you'll notice that the same applies to the app/doc menu.

Created attachment 152254 [details] [review]
[Overview] CSS tweaks to match mockups better

Adjust the size of the single-view images; while now technically
of the same size, we probably want to do something about the huge
difference in border luminance as well ...

Review of attachment 152089 [details] [review]:

Looks right; we recreate the workspaces when reentering so I can't see why we would reposition before hiding.

Review of attachment 152090 [details] [review]:

This is a nice cleanup.

Review of attachment 152092 [details] [review]:

::: js/ui/workspacesView.js
@@ +259,3 @@
+            return;
+        if (button == null)
+    _setButtonSensitivity: function(button, sensitive) {

Why are you testing the current button reactive state here?  Both the reactive and opacity property setters are noops if set to the same value.

Review of attachment 152243 [details] [review]:

Ideally this would be done through CSS; it's pretty easy to call .get_theme_node().get_length('-shell-workspace-spacing') or the like (or could just be 'spacing').

I understand that the constant mirrors the GRID_SPACING constant though so this isn't a blocker.

Review of attachment 152254 [details] [review]:

::: data/theme/gnome-shell.css
@@ +146,1 @@
 }

I'd tighten this rule a bit by saying ".workspaces-bar > StBoxLayout" for example; if someone later adds an internal box for another reason they might have to debug why it has a 5 spacing.

Created attachment 152481 [details] [review]
Make inactive workspace buttons insensitive

(In reply to comment #11)
> Review of attachment 152092 [details] [review]:
> Why are you testing the current button reactive state here?  Both the reactive
> and opacity property setters are noops if set to the same value.

OK, I wasn't aware of that - it's the same check that was used by the "old" 
workspace button.
Updated patch removes that check then ...

Created attachment 152483 [details] [review]
Make spacing between workspaces themable via CSS

(was: Reduce the gap between workspaces in linear view)

(In reply to comment #12)
> Review of attachment 152243 [details] [review]:
> I understand that the constant mirrors the GRID_SPACING constant though so this
> isn't a blocker.

Moved both constants to CSS.

Created attachment 152594 [details] [review]
"Fix" prelighting of the scroll bar handle

By default buttons fade from the hover to the normal state, by animating
the opacity of a copy of the previous border-image. This works as
expected for opaque and fully transparent pixels, but results in a
flickering effect for others.
Making StButton's fade effect work with partly transparent pixels is
hard, not using images with transparency is easy ...

Comment on attachment 152481 [details] [review]
Make inactive workspace buttons insensitive

Looks good, thanks!

Review of attachment 152483 [details] [review]:

One small comment

::: js/ui/workspacesView.js
@@ +273,3 @@
+                           Lang.bind(this, this._positionWorkspaces));
+        this.actor.connect('style-changed',
+        this.actor.style_class = "workspaces mosaic";

Why does only the MosaicView call _positionWorkspaces on style changes, but not the SingleView?

Review of attachment 152594 [details] [review]:

Sounds reasonable.

(In reply to comment #18)
> Review of attachment 152483 [details] [review]:
> Why does only the MosaicView call _positionWorkspaces on style changes, but not
> the SingleView?

MosaicView seems to require it, while SingleView's zoom-to-overlay animation breaks when _positionWorkspaces is called - not exactly sure about the why yet :(

Comment on attachment 152594 [details] [review]
"Fix" prelighting of the scroll bar handle

Attachment 152594 [details] pushed as e6902a1 - "Fix" prelighting of the scroll bar handle

Closing as the important part of attachment 152483 [details] [review] was pushed as cd78f1158cb. The moving of spacing constants to CSS should probably be done with some general cleanup in a separate bug.