Created attachment 157641 [details] [review]
Fix appIcon menu arrows

We were looking up the wrong property for the width, and then only
setting the foreground color if a void function returned TRUE.

Created attachment 157642 [details] [review]
remove some Big references from places that don't need them any more

Created attachment 157643 [details] [review]
[StDrawingArea] further CSS-ify StDrawingArea users

Make shell_draw_box_pointer() use CSS colors, and set the app well
menu arrow width based on its own CSS rather than its parent's.

Also, override get_preferred_height/width in StWidget to take CSS into
account so that, eg, an StDrawingArea with a CSS-specified minimum
width will request that width.

Created attachment 157644 [details] [review]
cleanup shell-menu a bit

to be squashed with next two commits

Created attachment 157645 [details] [review]
remove shell_menu_append_separator

since they already have the right behavior by virtue of not being reactive

to be squashed with previous and next commit

Created attachment 157646 [details] [review]
[ShellMenu] port from BigBox to StBoxLayout

Created attachment 157647 [details] [review]
Replace all remaining BigBoxes with StBoxLayouts or StBins

Also, remove a lot of cruft from genericDisplay.js leftover from
previous St-ifications, and remove the pre-gtk-2.16 hacks from the
status tray in panel.js (which are much less needed with the
nearly-all-black panel anyway).

Created attachment 157648 [details] [review]
Remove Big, which is no longer used

Review of attachment 157647 [details] [review]:

::: js/ui/genericDisplay.js
@@ -31,3 @@
-const DEFAULT_PADDING = 4;
-
-const ITEM_DISPLAY_ICON_SIZE = 48;

This is still used in docDisplay._createIcon ... removing it results into an exception 'can't convert size to an integer' and "More docs" being empty.

Created attachment 157712 [details] [review]
Replace all remaining BigBoxes with StBoxLayouts or StBins

put back the genericDisplay defines used by docDisplay

Review of attachment 157641 [details] [review]:

Looks good

Review of attachment 157642 [details] [review]:

Sweet!

Review of attachment 157643 [details] [review]:

Two minor comments

::: js/ui/appDisplay.js
@@ +671,2 @@
         let height = box.y2 - box.y1;
+        let [arrowMin, arrowWidth] = this._arrow.get_preferred_width(height);

Confusing to call one "Min" and one "Width"; I'd prefer to rename the second to "arrowNatural".

::: src/st/st-widget.c
@@ +647,3 @@
   gobject_class->finalize = st_widget_finalize;
+  actor_class->get_preferred_height = st_widget_get_preferred_height;
+  actor_class->get_preferred_width = st_widget_get_preferred_width;

This all looks right but it would be good as a separate git commit I think.

Review of attachment 157644 [details] [review]:

Just whitespace/indentation?  OK

Review of attachment 157645 [details] [review]:

::: js/ui/appDisplay.js
@@ +737,3 @@
         let bin = new St.Bin({ style_class: "app-well-menu-separator" });
     _appendSeparator: function () {
+        this._windowContainer.append(bin, Big.BoxPackFlags.NONE);

You can't pass Big.BoxPackFlags to a St.Bin.  But I assume this comes out OK in the squashing.

Review of attachment 157646 [details] [review]:

Looks ok

Review of attachment 157648 [details] [review]:

hooray!

(In reply to comment #13)
> +        let [arrowMin, arrowWidth] = this._arrow.get_preferred_width(height);
> 
> Confusing to call one "Min" and one "Width"; I'd prefer to rename the second to
> "arrowNatural".

I originally had it that way, but it made the rest of the function odd since we're using that value as the actual width that the arrow is going to be assigned, so "arrowWidth" is clearer than "arrowNatural".

Fixed by renaming "arrowMin" to "arrowMinWidth".

> +  actor_class->get_preferred_width = st_widget_get_preferred_width;
> 
> This all looks right but it would be good as a separate git commit I think.

done

committed the reviewed parts; note that you missed the
"Replace all remaining BigBoxes with StBoxLayouts or StBins" patch

Review of attachment 157712 [details] [review]:

Just one comment - I looked at all of the replacements and they looked right, but didn't really look to see if there were any that were missed.  The highest risk is in GenericDisplay since it's so complex, but I'm not too worried about it because we have a radical redesign scheduled there.

::: js/ui/appDisplay.js
@@ +1001,3 @@
         this._favorites = [];
+        this.actor = this._grid.actor;
+        this._grid = new WellGrid();

Won't this lose the horizontal centering?

Attachment 157648 [details] pushed as 4392033 - Remove Big, which is no longer used
Attachment 157712 [details] pushed as 2320c39 - Replace all remaining BigBoxes with StBoxLayouts or StBins