From 6d7c0c8f137bf1c4968e8777c7694fd2c1b7b392 Mon Sep 17 00:00:00 2001 From: Joe <33972521+hnb-ku@users.noreply.github.com> Date: Tue, 19 Mar 2019 18:39:38 +0800 Subject: [PATCH] REFACTOR: scroll-based mobile header switch This refactor addresses the following issues: 1- Moves all relevant logic to the discourse-topic component (matches desktop) 2- Fixes the flicker issue discussed here 3- Fixes a rare occurring issue where tags wrap to a third line if a topic has long category names and lots of tags 4- Fixes header icon jitter on iOS 5- Fixes an issue where sliding out user / hamburger menus on Android leaves the user in a mid-state with half a title and the header panel visible - swiping will now open the menus but have no effect on the header. 6- adds min-width to the small-logo to act as placeholder so that the title doesn't shift if the logo takes a while to load. Other than that, everything should look and act the same. --- .../components/discourse-topic.js.es6 | 115 ++++++++++-------- .../components/topic-progress.js.es6 | 41 ++----- .../stylesheets/common/base/header.scss | 11 +- app/assets/stylesheets/mobile/header.scss | 20 ++- 4 files changed, 99 insertions(+), 88 deletions(-) diff --git a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 index 4a112c7e200..fd4ce8fc1c7 100644 --- a/app/assets/javascripts/discourse/components/discourse-topic.js.es6 +++ b/app/assets/javascripts/discourse/components/discourse-topic.js.es6 @@ -5,6 +5,10 @@ import Scrolling from "discourse/mixins/scrolling"; import { selectedText } from "discourse/lib/utilities"; import { observes } from "ember-addons/ember-computed-decorators"; +const MOBILE_SCROLL_DIRECTION_CHECK_THROTTLE = 300; +// Small buffer so that very tiny scrolls don't trigger mobile header switch +const MOBILE_SCROLL_TOLERANCE = 5; + function highlight(postNumber) { const $contents = $(`#post_${postNumber} .topic-body`); @@ -12,9 +16,6 @@ function highlight(postNumber) { $contents.on("animationend", () => $contents.removeClass("highlighted")); } -// used to determine scroll direction on mobile -let lastScroll, scrollDirection, delta; - export default Ember.Component.extend(AddArchetypeClass, Scrolling, { userFilters: Ember.computed.alias("topic.userFilters"), classNameBindings: [ @@ -34,6 +35,9 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { _lastShowTopic: null, + mobileScrollDirection: null, + _mobileLastScroll: null, + @observes("enteredAt") _enteredTopic() { // Ember is supposed to only call observers when values change but something @@ -97,23 +101,6 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { this.appEvents.trigger("header:hide-topic"); } }); - // setup mobile scroll logo - if (this.site.mobileView) { - this.appEvents.on("topic:scrolled", offset => - this.mobileScrollGaurd(offset) - ); - // used to animate header contents on scroll - this.appEvents.on("header:show-topic", () => { - $("header.d-header") - .removeClass("scroll-up") - .addClass("scroll-down"); - }); - this.appEvents.on("header:hide-topic", () => { - $("header.d-header") - .removeClass("scroll-down") - .addClass("scroll-up"); - }); - } }, willDestroyElement() { @@ -129,11 +116,7 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { // this happens after route exit, stuff could have trickled in this.appEvents.trigger("header:hide-topic"); this.appEvents.off("post:highlight"); - // mobile scroll logo clean up. - if (this.site.mobileView) { - this.appEvents.off("topic:scrolled"); - $("header.d-header").removeClass("scroll-down scroll-up"); - } + this.appEvents.off("header:update-topic"); }, @observes("Discourse.hasFocus") @@ -148,17 +131,13 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { }, showTopicInHeader(topic, offset) { - // conditions for showing topic title in the header for mobile - if ( - this.site.mobileView && - scrollDirection !== "up" && - offset > this.dockAt - ) { - return true; - // condition for desktops - } else { - return offset > this.dockAt; - } + // On mobile, we show the header topic if the user has scrolled past the topic + // title and the current scroll direction is down + // On desktop the user only needs to scroll past the topic title. + return ( + offset > this.dockAt && + (!this.site.mobileView || this.mobileScrollDirection === "down") + ); }, // The user has scrolled the window, or it is finished rendering and ready for processing. scrolled() { @@ -193,25 +172,61 @@ export default Ember.Component.extend(AddArchetypeClass, Scrolling, { } } + // Since the user has scrolled, we need to check the scroll direction on mobile. + // We use throttle instead of debounce because we want the switch to occur + // at the start of the scroll. This feels a lot more snappy compared to waiting + // for the scroll to end if we debounce. + if (this.site.mobileView && this.hasScrolled) { + Ember.run.throttle( + this, + this._mobileScrollDirectionCheck, + offset, + MOBILE_SCROLL_DIRECTION_CHECK_THROTTLE + ); + } + // Trigger a scrolled event this.appEvents.trigger("topic:scrolled", offset); }, - // determines scroll direction, triggers header topic info on mobile - // and ensures that the switch happens only once per scroll direction change - mobileScrollGaurd(offset) { - // user hasn't scrolled past topic title. - if (offset < this.dockAt) return; + _mobileScrollDirectionCheck(offset) { + // Difference between this scroll and the one before it. + const delta = Math.floor(offset - this._mobileLastScroll); - delta = offset - lastScroll; - // 3px buffer so that the switch doesn't happen with tiny scrolls - if (delta > 3 && scrollDirection !== "down") { - scrollDirection = "down"; - this.appEvents.trigger("header:show-topic", this.topic); - } else if (delta < -3 && scrollDirection !== "up") { - scrollDirection = "up"; - this.appEvents.trigger("header:hide-topic"); + // This is a tiny scroll, so we ignore it. + if (delta <= MOBILE_SCROLL_TOLERANCE && delta >= -MOBILE_SCROLL_TOLERANCE) + return; + + const prevDirection = this.mobileScrollDirection; + const currDirection = delta > 0 ? "down" : "up"; + + if (currDirection !== prevDirection) { + this.set("mobileScrollDirection", currDirection); } - lastScroll = offset; + + // We store this to compare against it the next time the user scrolls + this._mobileLastScroll = Math.floor(offset); + + // If the user reaches the very bottom of the topic, we want to reset the + // scroll direction in order for the header to switch back. + const distanceToTopicBottom = Math.floor( + $("body").height() - offset - $(window).height() + ); + + // Not at the bottom yet + if (distanceToTopicBottom > 0) return; + + // We're at the bottom now, so we reset the direction. + this.set("mobileScrollDirection", null); + }, + + // We observe the scroll direction on mobile and if it's down, we show the topic + // in the header, otherwise, we hide it. + @observes("mobileScrollDirection") + toggleMobileHeaderTopic() { + return this.appEvents.trigger( + "header:update-topic", + this.mobileScrollDirection === "down" ? this.get("topic") : null + ); } }); diff --git a/app/assets/javascripts/discourse/components/topic-progress.js.es6 b/app/assets/javascripts/discourse/components/topic-progress.js.es6 index 3990124b668..ff1ffaa76c5 100644 --- a/app/assets/javascripts/discourse/components/topic-progress.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-progress.js.es6 @@ -1,4 +1,3 @@ -import { getOwner } from "discourse-common/lib/get-owner"; import { default as computed, observes @@ -155,16 +154,17 @@ export default Ember.Component.extend({ const $wrapper = this.$(); if (!$wrapper || $wrapper.length === 0) return; - const $html = $("html"), - offset = window.pageYOffset || $html.scrollTop(), - progressHeight = this.site.mobileView ? 0 : $("#topic-progress").height(), - maximumOffset = $("#topic-bottom").offset().top + progressHeight, - windowHeight = $(window).height(), - bodyHeight = $("body").height(), - composerHeight = $("#reply-control").height() || 0, - isDocked = offset >= maximumOffset - windowHeight + composerHeight, - bottom = bodyHeight - maximumOffset, - wrapperDir = $html.hasClass("rtl") ? "left" : "right"; + const $html = $("html"); + const offset = window.pageYOffset || $html.scrollTop(); + const progressHeight = this.site.mobileView + ? 0 + : $("#topic-progress").height(); + const maximumOffset = $("#topic-bottom").offset().top + progressHeight; + const windowHeight = $(window).height(); + const composerHeight = $("#reply-control").height() || 0; + const isDocked = offset >= maximumOffset - windowHeight + composerHeight; + const bottom = $("body").height() - maximumOffset; + const wrapperDir = $html.hasClass("rtl") ? "left" : "right"; if (composerHeight > 0) { $wrapper.css("bottom", isDocked ? bottom : composerHeight); @@ -180,25 +180,6 @@ export default Ember.Component.extend({ } else { $wrapper.css(wrapperDir, "1em"); } - - // switch mobile scroll logo at the very bottom of topics - if (this.site.mobileView) { - const isIOS = this.capabilities.isIOS, - switchHeight = bodyHeight - offset - windowHeight, - appEvents = getOwner(this).lookup("app-events:main"); - - if (isIOS && switchHeight < -10) { - // match elastic-scroll behaviour in iOS - setTimeout(function() { - appEvents.trigger("header:hide-topic"); - }, 300); - } else if (!isIOS && switchHeight < 5) { - // normal switch for everyone else - setTimeout(function() { - appEvents.trigger("header:hide-topic"); - }, 300); - } - } }, click(e) { diff --git a/app/assets/stylesheets/common/base/header.scss b/app/assets/stylesheets/common/base/header.scss index 5689ca6198d..ae71cd54c43 100644 --- a/app/assets/stylesheets/common/base/header.scss +++ b/app/assets/stylesheets/common/base/header.scss @@ -38,7 +38,7 @@ } .d-icon-home { - font-size: $font-up-5; + font-size: $font-up-6; } .panel { @@ -247,9 +247,12 @@ } .categories-wrapper { display: inline-flex; - max-width: 100%; - // only truncate the last category name. - > a:last-of-type { + // Protection against a very rare edge case in mobile header for topics with + // very long category names and lots of tags at the same time + max-width: 80%; + flex: 0 1 auto; + min-width: 1px; + .badge-wrapper { overflow: hidden; white-space: nowrap; text-overflow: ellipsis; diff --git a/app/assets/stylesheets/mobile/header.scss b/app/assets/stylesheets/mobile/header.scss index df9b599074e..f72f142e2bb 100644 --- a/app/assets/stylesheets/mobile/header.scss +++ b/app/assets/stylesheets/mobile/header.scss @@ -13,6 +13,11 @@ text-overflow: ellipsis; -webkit-animation: fadein 0.5s; animation: fadein 0.5s; + // This acts as a placeholder if for some reason the small logo takes a while + // to load - prevents topic title from shifting after the small logo loads. + // It's hardcoded to 36px because that's the width we use inline for the small + // logo in the home-logo widget. + min-width: 36px; a { display: block; width: 100%; @@ -44,13 +49,20 @@ button.sign-up-button { display: none; } - // styles for mobile scroll logo / topic + + // Hide header avatar + icons while topic title is visible in mobile header + .extra-info-wrapper + .panel { + flex: 0; + min-width: 0; + } + // Fade in header avatar + icons if topic title is not visible in mobile header .panel { - -webkit-animation: fadein 0.5s; animation: fadein 0.5s; } - &.scroll-down .panel { - display: none; + // A rendering bug in safari causes header SVGs to jitter after animations. + // translateZ() forces gpu rendering which fixes the issue. + .d-header-icons { + -webkit-transform: translateZ(0); } }