Created attachment 168768 [details] [review]
Calculate whether we need an expansion row for the banner correctly

The old calculation did not take into account the icon and the spacing
between columns. This resulted in the notifications that had a slightly
longer title/banner combination than could actually fit not being expandable.

The new calculation is done in _bannerBoxAllocate() so
that we don't need to hardcode which other elements are present.

Review of attachment 168768 [details] [review]:

::: js/ui/messageTray.js
@@ -324,3 @@
-    _styleChanged: function() {
-        let [has_spacing, spacing] = this.actor.get_theme_node().get_length('spacing-columns', false);
-        this._spacing = has_spacing ? spacing : 0;

I think it's better to still do this part from _styleChanged. That way the value is cached rather than needing to be looked up again each time.

@@ +364,3 @@
             this._bannerLabel.allocate(bannerBox, flags);
+            // Figure out if the notification should be expandable
+            // and add the expansion row after the allocation is done.

I'd say something like "Figure out if the banner text is too long to fit on one line, and if so, add it to the body. We can't do that from here though since that will force a relayout."

@@ +366,3 @@
+            // and add the expansion row after the allocation is done.
+            if (this._bannerBodyText && (bannerBox.x1 + bannerNatW > availWidth))
+                Mainloop.idle_add(Lang.bind(this,

This is in the wrong place; you need to do it in the case of the parent "if" being true as well (the case where it calls "this._bannerLabel.hide();". So it should happen outside the "if (titleBox.x2..." if, triggered by a flag set in the various cases

Created attachment 168839 [details] [review]
Calculate whether we need an expansion row for the banner correctly

The old calculation did not take into account the icon and the spacing
between columns. This resulted in the notifications that had a slightly
longer title/banner combination than could actually fit not being expandable.

The new calculation is done in _bannerBoxAllocate() so
that we don't need to hardcode which other elements are present.

Addressed all comments.

This patch still doesn't do anything in the case when the banner
doesn't fully fit, but this._bannerBody is false.