From 1972364d0f1b7907e726a8fce5a21a2d57f84b18 Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 10 Aug 2020 16:17:15 -0400 Subject: [PATCH] REFACTOR: Update the notification menu to remove scrolling (#10371) --- .../discourse/app/components/site-header.js | 27 +------ .../discourse/app/widgets/menu-panel.js | 2 +- .../app/widgets/quick-access-bookmarks.js | 11 +-- .../app/widgets/quick-access-messages.js | 8 +-- .../app/widgets/quick-access-notifications.js | 3 +- .../app/widgets/quick-access-panel.js | 65 +++++++---------- .../app/widgets/quick-access-profile.js | 4 +- .../discourse/app/widgets/user-menu.js | 23 ------ .../stylesheets/common/base/menu-panel.scss | 71 +++++++++++++++---- .../stylesheets/common/base/search-menu.scss | 4 ++ test/javascripts/widgets/user-menu-test.js | 2 +- 11 files changed, 98 insertions(+), 122 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index a31c16f375d..bb0baeea0c9 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/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"; - } else { + + let height; + if (this.site.mobileView) { height = winHeight - menuTop; } diff --git a/app/assets/javascripts/discourse/app/widgets/menu-panel.js b/app/assets/javascripts/discourse/app/widgets/menu-panel.js index cc8218e8b68..6d0ede4ffb8 100644 --- a/app/assets/javascripts/discourse/app/widgets/menu-panel.js +++ b/app/assets/javascripts/discourse/app/widgets/menu-panel.js @@ -37,7 +37,7 @@ createWidget("menu-panel", { tagName: "div.menu-panel", template: hbs`
-
+
{{yield}}
diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js b/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js index 75ca4816718..a0860ed95a3 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-bookmarks.js +++ b/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", - data: { - limit: this.estimateItemLimit() - } + cache: "false" }).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 }) => { diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-messages.js b/app/assets/javascripts/discourse/app/widgets/quick-access-messages.js index e8431bc3d27..b90d8abcd83 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-messages.js +++ b/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); }); }, diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-notifications.js b/app/assets/javascripts/discourse/app/widgets/quick-access-notifications.js index 2841da9a84d..6334242bdd5 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-notifications.js +++ b/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, - limit: this.estimateItemLimit() + silent: this.currentUser.enforcedSecondFactor }, { cacheKey: "recent-notifications" } ); diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js b/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js index b53868e0d49..6c568022007 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-panel.js +++ b/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()) { - items.push( - h( - "li.read.last.show-all", - this.attach("link", { - title: "view_all", - icon: "chevron-down", - href: this.showAllHref() - }) - ) + if (!this.hideBottomItems()) { + bottomItems.push( + this.attach("button", { + title: "view_all", + icon: "chevron-down", + className: "show-all", + url: 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() { diff --git a/app/assets/javascripts/discourse/app/widgets/quick-access-profile.js b/app/assets/javascripts/discourse/app/widgets/quick-access-profile.js index 278d5774775..c44a685d51a 100644 --- a/app/assets/javascripts/discourse/app/widgets/quick-access-profile.js +++ b/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() { diff --git a/app/assets/javascripts/discourse/app/widgets/user-menu.js b/app/assets/javascripts/discourse/app/widgets/user-menu.js index 6f4b5c323c7..04f89964854 100644 --- a/app/assets/javascripts/discourse/app/widgets/user-menu.js +++ b/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` - - ` -}); - 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; }, diff --git a/app/assets/stylesheets/common/base/menu-panel.scss b/app/assets/stylesheets/common/base/menu-panel.scss index 40e51d5b1bb..f69733a3cf2 100644 --- a/app/assets/stylesheets/common/base/menu-panel.scss +++ b/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-x: hidden; - max-height: 100vh; + overflow: hidden; + height: 100%; + } + + .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; - margin-top: -1px; + display: flex; + 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; - - a { - display: flex; - padding: 0.25em 0.5em; - } + align-self: flex-start; + width: 100%; .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); } diff --git a/app/assets/stylesheets/common/base/search-menu.scss b/app/assets/stylesheets/common/base/search-menu.scss index d74f5ea7224..32b2379b2f2 100644 --- a/app/assets/stylesheets/common/base/search-menu.scss +++ b/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; diff --git a/test/javascripts/widgets/user-menu-test.js b/test/javascripts/widgets/user-menu-test.js index c9d14032753..7cf3cfd258a 100644 --- a/test/javascripts/widgets/user-menu-test.js +++ b/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); } });