Created attachment 233878 [details] [review]
MessageTray: simplify image handling

Remove duplicate checks before calling unsetImage, and set properties
directly in the constructor.

Created attachment 233879 [details] [review]
MessageTray: restore symbolic icons to notification buttons

These were lost when we moved away with StIconType. The idea was that
apps needed to include -symbolic in their action IDs, but apps were
not updated, and it never makes sense to have non symbolic icons there,
so let's restore the previous behavior.

Created attachment 233880 [details] [review]
Notification: fix regression in notification image handling

We must set the image after calling .update(), because we're passing
clear: true and thus we're removing the image too.
Also, we need to specify an explicit icon size, or St.Icon will use
the default (48px)

Review of attachment 233878 [details] [review]:

OK.

Review of attachment 233879 [details] [review]:

::: js/ui/messageTray.js
@@ +662,3 @@
         button._actionId = id;
 
+        if (this._useActionIcons && Gtk.IconTheme.get_default().has_icon(id + '-symbolic')) {

function endsWith(str, suffix) {
    return str.slice(-suffix.length, 0) == suffix;
}

// Force symbolic icons on buttons.
let icon = id;
if (!endsWith(icon, '-symbolic')
    icon += '-symbolic';

Review of attachment 233880 [details] [review]:

OK.

Created attachment 233881 [details] [review]
MessageTray: don't show expanded content when the notification is collapsed

This fixes the image and scrollbars peeking through in banner mode,
because StTable wasn't unable to allocate them at the restricted height
imposed by CSS.

Review of attachment 233881 [details] [review]:

"wasn't unable"?

(In reply to comment #8)
> Review of attachment 233881 [details] [review]:
> 
> "wasn't unable"?

Typo, I meant wasn't able.

Review of attachment 233881 [details] [review]:

::: js/ui/components/telepathyClient.js
@@ +745,3 @@
         this._responseEntry = new St.Entry({ style_class: 'chat-response',
+                                             can_focus: true,
+                                             visible: this.expanded });

Couldn't we set this at setActionArea time?

Created attachment 233882 [details] [review]
MessageTray: restore symbolic icons to notification buttons

These were lost when we moved away with StIconType. The idea was that
apps needed to include -symbolic in their action IDs, but apps were
not updated, and it never makes sense to have non symbolic icons there,
so let's restore the previous behavior.

Created attachment 233884 [details] [review]
MessageTray: don't show expanded content when the notification is collapsed

This fixes the image and scrollbars peeking through in banner mode,
because StTable wasn't able to allocate them at the restricted height
imposed by CSS.

Review of attachment 233884 [details] [review]:

OK with one nit.

::: js/ui/messageTray.js
@@ +896,3 @@
         this.expanded = false;
+
+        // Show additional content that we keep hidden in banner mode

Hide additional content.

Review of attachment 233882 [details] [review]:

OK.

::: js/ui/messageTray.js
@@ +245,3 @@
 
+function strHasSuffix(string, suffix) {
+    return string.substr(-suffix.length) == suffix;

Huh, I never knew that substr handled negative indexes.

Attachment 233878 [details] pushed as 484ef5f - MessageTray: simplify image handling
Attachment 233880 [details] pushed as 45fde48 - Notification: fix regression in notification image handling
Attachment 233882 [details] pushed as 9aa84ec - MessageTray: restore symbolic icons to notification buttons
Attachment 233884 [details] pushed as 91a6520 - MessageTray: don't show expanded content when the notification is collapsed

*** Bug 687682 has been marked as a duplicate of this bug. ***