notification: Limit max height and enable vscroll

files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js

@@ -74,7 +74,7 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet {
     }
 
     _openMenu() {
-        this._update_timestamp();
+        this._update_notification_status();
         this.menu.toggle();
     }
 
@@ -310,13 +310,18 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet {
         Util.spawnCommandLine("cinnamon-settings notifications");
     }
 
-    _update_timestamp() {
+    _update_notification_status() {
         let len = this.notifications.length;
         if (len > 0) {
             for (let i = 0; i < len; i++) {
                 let notification = this.notifications[i];
                 let orig_time = notification._timestamp;
                 notification._timeLabel.clutter_text.set_markup(timeify(orig_time));
+
+                // Disable body scroll for notifications and allow the notification widget to expand.
+                // This is to avoid having notifications with scrollable bodies inside the scrollable notification menu,
+                // which would be a bad user experience.
+                notification.setBodyExpand(true);
             }
         }
     }

js/ui/messageTray.js

@@ -392,38 +392,34 @@ var Notification = class Notification {
     _setBodyArea(text, allowMarkup) {
         if (text) {
             if (!this._scrollArea) {
-                /* FIXME: vscroll should be enabled
-                 * -vfade covers too much for this size of scrollable
-                 * -scrollview min-height is broken inside tray with a scrollview
-                 *
-                 * TODO: when scrollable:
-                 *
-                 * applet connects to this signal to enable captured-event passthru so you can grab the scrollbar:
-                 * let vscroll = this._scrollArea.get_vscroll_bar();
-                 * vscroll.connect('scroll-start', () => { this.emit('scrolling-changed', true) });
-                 * vscroll.connect('scroll-stop', () => { this.emit('scrolling-changed', false) });
-                 *
-                 * `enable_mouse_scrolling` makes it difficult to scroll when there are many notifications
-                 * in the tray because most of the area is these smaller scrollviews which capture the event.
-                 * ideally, this should only be disabled when the notification is in the tray and there are
-                 * many notifications.
-                 */
                 this._scrollArea = new St.ScrollView({
                     name: 'notification-scrollview',
-                    vscrollbar_policy: St.PolicyType.NEVER,
                     hscrollbar_policy: St.PolicyType.NEVER,
-                    enable_mouse_scrolling: false/*,
-                                                       style_class: 'vfade'*/ });
+                });
+
+                this._scrollArea.set_clip_to_allocation(true);
+
+                let vscroll = this._scrollArea.get_vscroll_bar();
+                if (vscroll) {
+                    vscroll.connect('scroll-start', () => { this.emit('scrolling-changed', true) });
+                    vscroll.connect('scroll-stop', () => { this.emit('scrolling-changed', false) });
+                }
+
+                this.setBodyExpand(false, false);
 
                 this._table.add(this._scrollArea, {
                     row: 1,
-                    col: 2
+                    col: 2,
+                    y_expand: false,
+                    y_fill: false,
+                    y_align: St.Align.START
                 });
 
                 let content = new St.BoxLayout({
                     name: 'notification-body',
                     vertical: true
                 });
+
                 this._scrollArea.add_actor(content);
 
                 // body label
@@ -463,14 +459,26 @@ var Notification = class Notification {
             adjustment.value = adjustment.upper;
     }
 
+    setBodyExpand(enabled, updateLayout = true) {
+        this._bodyExpandEnabled = enabled;
+        if (this._scrollArea) {
+            this._scrollArea.vscrollbar_policy = enabled ?  St.PolicyType.NEVER : St.PolicyType.AUTOMATIC;
+            this._scrollArea.enable_mouse_scrolling = !enabled;
+            this._table.set_style(`max-height: ${enabled ? 'none' : '40em'};`);
+        }
+        if (updateLayout) {
+            this._updateLayout();
+        }
+    }
+
     _updateLayout() {
         if (this._imageBin || this._scrollArea || this._actionArea) {
             this._table.add_style_class_name('multi-line-notification');
         } else {
             this._table.remove_style_class_name('multi-line-notification');
         }
 
-        if (this._imageBin) {
+        if (this._imageBin && !this._bodyExpandEnabled) {
             this._table.add_style_class_name('notification-with-image');
         } else {
             this._table.remove_style_class_name('notification-with-image');
@@ -504,7 +512,8 @@ var Notification = class Notification {
             x_expand: false,
             y_expand: false,
             x_fill: false,
-            y_fill: false
+            y_fill: false,
+            y_align: St.Align.START
         });
         this._updateLayout();
     }

src/st/st-scroll-bar.c

@@ -659,6 +659,10 @@ handle_button_press_event_cb (ClutterActor       *actor,
   priv->x_origin += clutter_actor_get_x (priv->trough);
   priv->y_origin += clutter_actor_get_y (priv->trough);
 
+  /* Clean up any stale grab before creating a new one. */
+  if (priv->grab_device)
+      stop_scrolling (bar);
+
   g_assert (!priv->grab_device);
 
   clutter_input_device_grab (device, priv->handle);