REFACTOR: Update the notification menu to remove scrolling (#10371)

app/assets/javascripts/discourse/app/components/site-header.js

@@ -9,8 +9,6 @@ import PanEvents, {
 } from "discourse/mixins/pan-events";
 import { topicTitleDecorators } from "discourse/components/topic-title";
 
-const PANEL_BODY_MARGIN = 30;
-
 const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
   widget: "header",
   docAt: null,
@@ -311,8 +309,6 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
       }
 
       const $panelBody = $(".panel-body", $panel);
-      // 2 pixel fudge allows for firefox subpixel sizing stuff causing scrollbar
-      let contentHeight = $(".panel-body-contents", $panel).height() + 2;
 
       // We use a mutationObserver to check for style changes, so it's important
       // we don't set it if it doesn't change. Same goes for the $panelBody!
@@ -330,22 +326,6 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
           $panel.css({ top: "100%", height: "auto" });
         }
 
-        // adjust panel height
-        const fullHeight = $window.height();
-        const offsetTop = $panel.offset().top;
-        const scrollTop = $window.scrollTop();
-
-        if (
-          contentHeight + (offsetTop - scrollTop) + PANEL_BODY_MARGIN >
-            fullHeight ||
-          this.site.mobileView
-        ) {
-          contentHeight =
-            fullHeight - (offsetTop - scrollTop) - PANEL_BODY_MARGIN;
-        }
-        if ($panelBody.height() !== contentHeight) {
-          $panelBody.height(contentHeight);
-        }
         $("body").addClass("drop-down-mode");
       } else {
         if (this.site.mobileView) {
@@ -354,15 +334,14 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, {
 
         const menuTop = this.site.mobileView ? headerTop() : headerHeight();
 
-        let height;
         const winHeightOffset = 16;
         let initialWinHeight = window.innerHeight
           ? window.innerHeight
           : $(window).height();
         const winHeight = initialWinHeight - winHeightOffset;
-        if (menuTop + contentHeight < winHeight && !this.site.mobileView) {
+
-          height = contentHeight + "px";
+        let height;
-        } else {
+        if (this.site.mobileView) {
           height = winHeight - menuTop;
         }
 

app/assets/javascripts/discourse/app/widgets/menu-panel.js

@@ -37,7 +37,7 @@ createWidget("menu-panel", {
   tagName: "div.menu-panel",
   template: hbs`
     <div class='panel-body'>
-      <div class='panel-body-contents clearfix'>
+      <div class='panel-body-contents'>
         {{yield}}
       </div>
     </div>

app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js

@@ -10,11 +10,6 @@ const ICON = "bookmark";
 createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
   buildKey: () => "quick-access-bookmarks",
 
-  hasMore() {
-    // Always show the button to the bookmarks page.
-    return true;
-  },
-
   showAllHref() {
     return `${this.attrs.path}/activity/bookmarks`;
   },
@@ -49,10 +44,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
 
   loadBookmarksWithReminders() {
     return ajax(`/u/${this.currentUser.username}/bookmarks.json`, {
-      cache: "false",
+      cache: "false"
-      data: {
-        limit: this.estimateItemLimit()
-      }
     }).then(result => {
       result = result.user_bookmark_list;
 
@@ -71,7 +63,6 @@ createWidgetFrom(QuickAccessPanel, "quick-access-bookmarks", {
       data: {
         username: this.currentUser.username,
         filter: UserAction.TYPES.bookmarks,
-        limit: this.estimateItemLimit(),
         no_results_help_key: "user_activity.no_bookmarks"
       }
     }).then(({ user_actions, no_results_help }) => {

app/assets/javascripts/discourse/app/widgets/quick-access-messages.js

@@ -24,12 +24,6 @@ createWidgetFrom(QuickAccessPanel, "quick-access-messages", {
   buildKey: () => "quick-access-messages",
   emptyStatePlaceholderItemKey: "choose_topic.none_found",
 
-  hasMore() {
-    // Always show the button to the messages page for composing, archiving,
-    // etc.
-    return true;
-  },
-
   showAllHref() {
     return `${this.attrs.path}/messages`;
   },
@@ -40,7 +34,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-messages", {
         filter: `topics/private-messages/${this.currentUser.username_lower}`
       })
       .then(({ topic_list }) => {
-        return topic_list.topics.map(toItem).slice(0, this.estimateItemLimit());
+        return topic_list.topics.map(toItem);
       });
   },
 

app/assets/javascripts/discourse/app/widgets/quick-access-notifications.js

@@ -46,8 +46,7 @@ createWidgetFrom(QuickAccessPanel, "quick-access-notifications", {
       "notification",
       {
         recent: true,
-        silent: this.currentUser.enforcedSecondFactor,
+        silent: this.currentUser.enforcedSecondFactor
-        limit: this.estimateItemLimit()
       },
       { cacheKey: "recent-notifications" }
     );

app/assets/javascripts/discourse/app/widgets/quick-access-panel.js

@@ -2,15 +2,8 @@ import I18n from "I18n";
 import Session from "discourse/models/session";
 import { createWidget } from "discourse/widgets/widget";
 import { h } from "virtual-dom";
-import { headerHeight } from "discourse/components/site-header";
 import { Promise } from "rsvp";
 
-// even a 2 liner notification should be under 50px in default view
-const AVERAGE_ITEM_HEIGHT = 50;
-
-// our UX usually carries about 100px of padding around the notification excluding header
-const PADDING = 100;
-
 /**
  * This tries to enforce a consistent flow of fetching, caching, refreshing,
  * and rendering for "quick access items".
@@ -31,6 +24,10 @@ export default createWidget("quick-access-panel", {
     return Promise.resolve();
   },
 
+  hideBottomItems() {
+    return false;
+  },
+
   hasUnread() {
     return false;
   },
@@ -39,10 +36,6 @@ export default createWidget("quick-access-panel", {
     return "";
   },
 
-  hasMore() {
-    return this.getItems().length >= this.estimateItemLimit();
-  },
-
   findNewItems() {
     return Promise.resolve([]);
   },
@@ -69,23 +62,6 @@ export default createWidget("quick-access-panel", {
     });
   },
 
-  estimateItemLimit() {
-    // Estimate (poorly) the amount of notifications to return.
-    let limit = Math.round(
-      ($(window).height() - headerHeight() - PADDING) / AVERAGE_ITEM_HEIGHT
-    );
-
-    // We REALLY don't want to be asking for negative counts of notifications
-    // less than 5 is also not that useful.
-    if (limit < 5) {
-      limit = 5;
-    } else if (limit > 40) {
-      limit = 40;
-    }
-
-    return limit;
-  },
-
   refreshNotifications(state) {
     if (this.loading) {
       return;
@@ -119,24 +95,35 @@ export default createWidget("quick-access-panel", {
       return [h("div.spinner-container", h("div.spinner"))];
     }
 
+    let bottomItems = [];
     const items = this.getItems().length
       ? this.getItems().map(item => this.itemHtml(item))
       : [this.emptyStatePlaceholderItem()];
 
-    if (this.hasMore()) {
+    if (!this.hideBottomItems()) {
-      items.push(
+      bottomItems.push(
-        h(
+        this.attach("button", {
-          "li.read.last.show-all",
+          title: "view_all",
-          this.attach("link", {
+          icon: "chevron-down",
-            title: "view_all",
+          className: "show-all",
-            icon: "chevron-down",
+          url: this.showAllHref()
-            href: this.showAllHref()
+        })
-          })
-        )
       );
     }
 
-    return [h("ul", items)];
+    if (this.hasUnread()) {
+      bottomItems.push(
+        this.attach("button", {
+          title: "user.dismiss_notifications_tooltip",
+          icon: "check",
+          label: "user.dismiss",
+          className: "notifications-dismiss",
+          action: "dismissNotifications"
+        })
+      );
+    }
+
+    return [h("ul", items), h("div.panel-body-bottom", bottomItems)];
   },
 
   getItems() {

app/assets/javascripts/discourse/app/widgets/quick-access-profile.js

@@ -14,9 +14,9 @@ createWidgetFrom(QuickAccessPanel, "quick-access-profile", {
 
   buildKey: () => "quick-access-profile",
 
-  hasMore() {
+  hideBottomItems() {
     // Never show the button to the full profile page.
-    return false;
+    return true;
   },
 
   findNewItems() {

app/assets/javascripts/discourse/app/widgets/user-menu.js

@@ -2,7 +2,6 @@ import { later } from "@ember/runloop";
 import { createWidget } from "discourse/widgets/widget";
 import { h } from "virtual-dom";
 import { formatUsername } from "discourse/lib/utilities";
-import hbs from "discourse/widgets/hbs-compiler";
 
 const UserMenuAction = {
   QUICK_ACCESS: "quickAccess"
@@ -145,23 +144,6 @@ createWidget("user-menu-links", {
   }
 });
 
-createWidget("user-menu-dismiss-link", {
-  tagName: "div.dismiss-link",
-
-  template: hbs`
-    <ul class='menu-links'>
-      <li>
-        {{link action="dismissNotifications"
-          className="dismiss"
-          tabindex="0"
-          icon="check"
-          label="user.dismiss"
-          title="user.dismiss_notifications_tooltip"}}
-      </li>
-    </ul>
-  `
-});
-
 export default createWidget("user-menu", {
   tagName: "div.user-menu",
   buildKey: () => "user-menu",
@@ -191,11 +173,6 @@ export default createWidget("user-menu", {
       this.quickAccessPanel(path)
     ];
 
-    if (this.state.hasUnread) {
-      result.push(h("hr.bottom-area"));
-      result.push(this.attach("user-menu-dismiss-link"));
-    }
-
     return result;
   },
 

app/assets/stylesheets/common/base/menu-panel.scss

@@ -19,6 +19,7 @@
   // positions are relative to the .d-header .panel div
   top: 100%; // directly underneath .panel
   right: -10px; // 10px to the right of .panel - adjust as needed
+  max-height: 80vh;
 }
 
 .menu-panel {
@@ -28,7 +29,9 @@
   z-index: z("header");
   padding: 0.5em;
   width: 300px;
-
+  overflow: hidden;
+  display: flex;
+  flex-direction: column;
   hr {
     margin: 3px 0;
   }
@@ -45,10 +48,41 @@
   }
 
   .panel-body {
+    display: flex;
     touch-action: pan-y pinch-zoom;
-    overflow-y: auto;
+    overflow: hidden;
-    overflow-x: hidden;
+    height: 100%;
-    max-height: 100vh;
+  }
+
+  .panel-body-contents {
+    max-height: 100%;
+    width: 100%;
+    display: flex;
+    flex-direction: column;
+  }
+
+  .panel-body-bottom {
+    display: flex;
+    flex: 1 0 0%; // safari height fix
+    margin-top: 0.5em;
+    .show-all {
+      flex: 1 1 auto;
+      button {
+        width: 100%;
+      }
+    }
+    .notifications-dismiss {
+      margin-left: 0.5em;
+    }
+
+    button {
+      background-color: var(--primary-very-low);
+      color: var(--primary-high);
+      &:hover {
+        background: var(--primary-low);
+        color: var(--primary);
+      }
+    }
   }
 
   .badge-notification {
@@ -153,11 +187,13 @@
 .user-menu {
   .quick-access-panel {
     width: 100%;
-    display: table;
+    display: flex;
-    margin-top: -1px;
+    flex-direction: column;
+    min-height: 0;
+    max-height: 100%;
     border-top: 1px solid var(--primary-low);
     padding-top: 0.5em;
-
+    margin-top: -1px;
     h3 {
       padding: 0 0.4em;
       font-weight: bold;
@@ -172,8 +208,17 @@
       color: var(--primary-high);
     }
 
+    ul {
+      display: flex;
+      flex-flow: column wrap;
+      overflow: hidden;
+      max-height: 100%;
+    }
+
     li {
       background-color: var(--tertiary-low);
+      box-sizing: border-box;
+      list-style-type: none;
 
       // This is until other languages remove the HTML from within
       // notifications. It can then be removed
@@ -192,7 +237,8 @@
       }
 
       a {
-        padding: 0;
+        display: flex;
+        padding: 0.25em 0.5em;
 
         > div {
           overflow: hidden; // clears the text from wrapping below icons
@@ -215,11 +261,8 @@
     }
     li:not(.show-all) {
       padding: 0;
-
+      align-self: flex-start;
-      a {
+      width: 100%;
-        display: flex;
-        padding: 0.25em 0.5em;
-      }
       .d-icon {
         padding-top: 0.2em;
         margin-right: 0.5em;
@@ -264,6 +307,8 @@
     &.quick-access-profile {
       li:not(.show-all) a {
         color: var(--primary);
+        display: flex;
+        padding: 0.25em 0.5em;
         .d-icon {
           color: var(--primary-medium);
         }

app/assets/stylesheets/common/base/search-menu.scss

@@ -1,4 +1,8 @@
 .search-menu {
+  .menu-panel .panel-body-contents {
+    overflow-y: auto;
+  }
+
   .search-input {
     position: relative;
     padding: 0.5em 3px;

test/javascripts/widgets/user-menu-test.js

@@ -13,7 +13,7 @@ widgetTest("basics", {
     assert.ok(find(".user-notifications-link").length);
     assert.ok(find(".user-bookmarks-link").length);
     assert.ok(find(".quick-access-panel").length);
-    assert.ok(find(".dismiss-link").length);
+    assert.ok(find(".notifications-dismiss").length);
   }
 });