From da5841de0b2b60ceaa2f2602d7231e7393a74e66 Mon Sep 17 00:00:00 2001 From: Kris Date: Mon, 19 Oct 2020 17:26:38 -0400 Subject: [PATCH] REFACTOR: Remove position fixed from the header and use sticky instead (#10781) This removes fixed positioning from d-header and the topic timeline. Plugins, themes and components that use the above/below header plugin outlet will likely need some margin/padding adjustments. --- .../discourse/app/components/site-header.js | 4 +- .../discourse/app/lib/offset-calculator.js | 18 ++-- .../discourse/app/templates/topic.hbs | 92 ++++++++++--------- .../discourse/app/widgets/topic-timeline.js | 6 -- .../stylesheets/common/base/discourse.scss | 6 +- .../stylesheets/common/base/header.scss | 14 +-- app/assets/stylesheets/common/base/topic.scss | 45 +++++++++ .../stylesheets/common/foundation/mixins.scss | 5 + .../stylesheets/common/topic-timeline.scss | 21 ++--- app/assets/stylesheets/desktop/discourse.scss | 3 - app/assets/stylesheets/desktop/header.scss | 5 - app/assets/stylesheets/desktop/topic.scss | 12 +-- app/assets/stylesheets/mobile/header.scss | 1 - 13 files changed, 132 insertions(+), 100 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index 3784dcf4b46..f0c8f8fdb67 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -373,7 +373,9 @@ const SiteHeaderComponent = MountWidget.extend(Docking, PanEvents, { }, }); -export default SiteHeaderComponent; +export default SiteHeaderComponent.extend({ + classNames: ["d-header-wrap"], +}); export function headerHeight() { const $header = $("header.d-header"); diff --git a/app/assets/javascripts/discourse/app/lib/offset-calculator.js b/app/assets/javascripts/discourse/app/lib/offset-calculator.js index c8e43d63029..efa258fc70c 100644 --- a/app/assets/javascripts/discourse/app/lib/offset-calculator.js +++ b/app/assets/javascripts/discourse/app/lib/offset-calculator.js @@ -3,31 +3,29 @@ export function scrollTopFor(y) { } export function minimumOffset() { - const $header = $("header.d-header"); - const headerHeight = $header.outerHeight(true) || 0; - const headerPositionTop = $header.position().top; - return headerHeight + headerPositionTop; + const header = document.querySelector("header.d-header"); + return header ? header.offsetHeight : 0; } export default function offsetCalculator() { const min = minimumOffset(); // on mobile, just use the header - if ($("html").hasClass("mobile-view")) { + if (document.querySelector("html").classList.contains("mobile-view")) { return min; } - const $window = $(window); - const windowHeight = $window.height(); - const documentHeight = $(document).height(); - const topicBottomOffsetTop = $("#topic-bottom").offset().top; + const windowHeight = window.innerHeight; + const documentHeight = document.body.clientHeight; + const topicBottomOffsetTop = document.getElementById("topic-bottom") + .offsetTop; // the footer is bigger than the window, we can scroll down past the last post if (documentHeight - windowHeight > topicBottomOffsetTop) { return min; } - const scrollTop = $window.scrollTop(); + const scrollTop = window.scrollY; const visibleBottomHeight = scrollTop + windowHeight - topicBottomOffsetTop; if (visibleBottomHeight > 0) { diff --git a/app/assets/javascripts/discourse/app/templates/topic.hbs b/app/assets/javascripts/discourse/app/templates/topic.hbs index 0490f39b162..de1409211fe 100644 --- a/app/assets/javascripts/discourse/app/templates/topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/topic.hbs @@ -128,7 +128,7 @@ }} - {{#topic-navigation topic=model jumpToDate=(action "jumpToDate") jumpToIndex=(action "jumpToIndex") as |info|}} + {{#topic-navigation class="topic-navigation" topic=model jumpToDate=(action "jumpToDate") jumpToIndex=(action "jumpToIndex") as |info|}} {{#if info.renderTimeline}} {{topic-timeline topic=model @@ -299,41 +299,6 @@ categoryId=model.topic_timer.category_id removeTopicTimer=(action "removeTopicTimer" model.topic_timer.status_type "topic_timer")}} - {{#if session.showSignupCta}} - {{! replace "Log In to Reply" with the infobox }} - {{signup-cta}} - {{else}} - {{#if currentUser}} - {{plugin-outlet name="topic-above-footer-buttons" args=(hash model=model)}} - - {{topic-footer-buttons - topic=model - toggleMultiSelect=(action "toggleMultiSelect") - showTopicSlowModeUpdate=(route-action "showTopicSlowModeUpdate") - deleteTopic=(action "deleteTopic") - recoverTopic=(action "recoverTopic") - toggleClosed=(action "toggleClosed") - toggleArchived=(action "toggleArchived") - toggleVisibility=(action "toggleVisibility") - showTopicStatusUpdate=(route-action "showTopicStatusUpdate") - showFeatureTopic=(route-action "showFeatureTopic") - showChangeTimestamp=(route-action "showChangeTimestamp") - resetBumpDate=(action "resetBumpDate") - convertToPublicTopic=(action "convertToPublicTopic") - convertToPrivateMessage=(action "convertToPrivateMessage") - toggleBookmark=(action "toggleBookmark") - showFlagTopic=(route-action "showFlagTopic") - toggleArchiveMessage=(action "toggleArchiveMessage") - editFirstPost=(action "editFirstPost") - deferTopic=(action "deferTopic") - replyToPost=(action "replyToPost")}} - {{else}} - - {{/if}} - {{/if}} - {{#if showSelectedPostsAtBottom}}
{{selected-posts @@ -353,15 +318,6 @@
{{/if}} - {{plugin-outlet name="topic-above-suggested" args=(hash model=model)}} -
- {{#if model.relatedMessages.length}} - {{related-messages topic=model}} - {{/if}} - {{#if model.suggestedTopics.length}} - {{suggested-topics topic=model}} - {{/if}} -
{{/if}} {{/conditional-loading-spinner}} @@ -369,6 +325,52 @@ + {{#if loadedAllPosts}} + {{#if session.showSignupCta}} + {{! replace "Log In to Reply" with the infobox }} + {{signup-cta}} + {{else}} + {{#if currentUser}} + {{plugin-outlet name="topic-above-footer-buttons" args=(hash model=model)}} + + {{topic-footer-buttons + topic=model + toggleMultiSelect=(action "toggleMultiSelect") + showTopicSlowModeUpdate=(route-action "showTopicSlowModeUpdate") + deleteTopic=(action "deleteTopic") + recoverTopic=(action "recoverTopic") + toggleClosed=(action "toggleClosed") + toggleArchived=(action "toggleArchived") + toggleVisibility=(action "toggleVisibility") + showTopicStatusUpdate=(route-action "showTopicStatusUpdate") + showFeatureTopic=(route-action "showFeatureTopic") + showChangeTimestamp=(route-action "showChangeTimestamp") + resetBumpDate=(action "resetBumpDate") + convertToPublicTopic=(action "convertToPublicTopic") + convertToPrivateMessage=(action "convertToPrivateMessage") + toggleBookmark=(action "toggleBookmark") + showFlagTopic=(route-action "showFlagTopic") + toggleArchiveMessage=(action "toggleArchiveMessage") + editFirstPost=(action "editFirstPost") + deferTopic=(action "deferTopic") + replyToPost=(action "replyToPost")}} + {{else}} + + {{/if}} + {{/if}} + + {{plugin-outlet name="topic-above-suggested" args=(hash model=model)}} +
+ {{#if model.relatedMessages.length}} + {{related-messages topic=model}} + {{/if}} + {{#if model.suggestedTopics.length}} + {{suggested-topics topic=model}} + {{/if}} +
+ {{/if}} {{else}}
{{#conditional-loading-spinner condition=noErrorYet}} diff --git a/app/assets/javascripts/discourse/app/widgets/topic-timeline.js b/app/assets/javascripts/discourse/app/widgets/topic-timeline.js index 29361983576..ba21c6eb8dc 100644 --- a/app/assets/javascripts/discourse/app/widgets/topic-timeline.js +++ b/app/assets/javascripts/discourse/app/widgets/topic-timeline.js @@ -320,12 +320,6 @@ createWidget("topic-timeline-container", { } }, - buildAttributes(attrs) { - if (attrs.top) { - return { style: `top: ${attrs.top}px` }; - } - }, - html(attrs) { return this.attach("topic-timeline", attrs); }, diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index d185bc5b091..69ebcbb25b9 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -580,7 +580,11 @@ table { } // Special elements -// Special elements + +#main-outlet { + padding-top: 1.8em; +} + #main { img.avatar { &.header { diff --git a/app/assets/stylesheets/common/base/header.scss b/app/assets/stylesheets/common/base/header.scss index 1d22adb5d97..4c7db280ebc 100644 --- a/app/assets/stylesheets/common/base/header.scss +++ b/app/assets/stylesheets/common/base/header.scss @@ -1,12 +1,17 @@ +.d-header-wrap { + @include sticky; + top: 0; + z-index: z("header"); +} + .d-header { display: flex; align-items: center; width: 100%; - position: absolute; - top: 0; z-index: z("header"); background-color: var(--header_background); box-shadow: shadow("header"); + backface-visibility: hidden; /** do magic for scrolling performance **/ > .wrap { width: calc(100% - 16px); // accommodates for 8px vertical padding @@ -35,11 +40,6 @@ } } - .docked & { - position: fixed; - backface-visibility: hidden; /** do magic for scrolling performance **/ - } - .title { display: flex; align-items: center; diff --git a/app/assets/stylesheets/common/base/topic.scss b/app/assets/stylesheets/common/base/topic.scss index 662baa329ee..68975dbea86 100644 --- a/app/assets/stylesheets/common/base/topic.scss +++ b/app/assets/stylesheets/common/base/topic.scss @@ -16,6 +16,51 @@ } } +.container.posts { + display: grid; + grid-template-areas: "posts timeline"; + grid-template-columns: auto auto; + > .row { + grid-area: posts; + max-width: 100%; + overflow: hidden; + } + + .timeline-container { + margin-left: unset !important; + /* This is a temporary override to ease the transition + to the sticky position timeline for themes with custom timeline positioning. + Without this those themes would render topics unreadable. */ + } + + // timeline + @media screen and (min-width: 925px) { + // at 925px viewport width and above the timeline is visible (see topic-navigation.js) + .topic-navigation { + grid-area: timeline; + align-self: start; + @include sticky; + top: 6em; + margin-left: 1em; + z-index: z("timeline"); + } + } + + // progress bar + @media screen and (max-width: 924px) { + // at 924px viewport width and below the progress bar is visible (see topic-navigation.js) + grid-template-areas: "posts posts"; + .timeline-container:not(.timeline-fullscreen) { + display: none; // hiding this because sometimes the JS switch lags and causes layout issues + } + .timeline-container { + .timeline-scroller-content { + position: relative; + } + } + } +} + .progress-back-container { z-index: z("dropdown"); margin-right: 0; diff --git a/app/assets/stylesheets/common/foundation/mixins.scss b/app/assets/stylesheets/common/foundation/mixins.scss index 6de8e91060d..92fabcf0cc0 100644 --- a/app/assets/stylesheets/common/foundation/mixins.scss +++ b/app/assets/stylesheets/common/foundation/mixins.scss @@ -75,6 +75,11 @@ $breakpoints: ( // // -------------------------------------------------- +@mixin sticky { + position: -webkit-sticky; + position: sticky; +} + // Unselectable (avoids unwanted selections with iPad, touch laptops, etc) @mixin user-select($mode) { diff --git a/app/assets/stylesheets/common/topic-timeline.scss b/app/assets/stylesheets/common/topic-timeline.scss index 98b69dfc9f4..eba6f2b9dac 100644 --- a/app/assets/stylesheets/common/topic-timeline.scss +++ b/app/assets/stylesheets/common/topic-timeline.scss @@ -5,22 +5,8 @@ .timeline-container { box-sizing: border-box; z-index: z("timeline"); - margin-left: 757px; - - @include breakpoint(extra-large, min-width) { - margin-left: 820px; - } - @media all and (min-width: 1250px) { - margin-left: 900px; - } - - position: fixed; -webkit-transform: translate3d(0, 0, 0); - &.timeline-docked { - position: absolute; - } - &.timeline-docked-bottom { .timeline-footer-controls { opacity: 0; @@ -173,7 +159,6 @@ } .topic-timeline { - margin-left: 3em; transition: opacity 0.2s ease-in; touch-action: none; @@ -181,6 +166,11 @@ margin-bottom: 1em; } + .timeline-date-wrapper { + max-width: 9em; + overflow-wrap: anywhere; + } + .timeline-footer-controls { margin-top: 1.5em; transition: opacity 0.2s ease-in; @@ -228,6 +218,7 @@ .timeline-scroller-content { padding-left: 1em; + position: absolute; // prevents text length from impacting width } .timeline-ago { diff --git a/app/assets/stylesheets/desktop/discourse.scss b/app/assets/stylesheets/desktop/discourse.scss index ebcee9c726f..ecc79e17d42 100644 --- a/app/assets/stylesheets/desktop/discourse.scss +++ b/app/assets/stylesheets/desktop/discourse.scss @@ -168,9 +168,6 @@ input { } /* bootstrap columns */ -.row { - @include clearfix; -} .offset { &2 { diff --git a/app/assets/stylesheets/desktop/header.scss b/app/assets/stylesheets/desktop/header.scss index 9918bfa8ac0..f4a37592ec2 100644 --- a/app/assets/stylesheets/desktop/header.scss +++ b/app/assets/stylesheets/desktop/header.scss @@ -5,7 +5,6 @@ .d-header { left: 0; height: 4em; - margin-bottom: 15px; #site-logo { height: 2.667em; // 40px with default 15px font size } @@ -32,10 +31,6 @@ position: relative; } -#main-outlet { - padding-top: 5.8572em; -} - .search-link .blurb { color: var(--secondary-medium); display: block; diff --git a/app/assets/stylesheets/desktop/topic.scss b/app/assets/stylesheets/desktop/topic.scss index 3c175e4e2cc..bbd65991e02 100644 --- a/app/assets/stylesheets/desktop/topic.scss +++ b/app/assets/stylesheets/desktop/topic.scss @@ -207,13 +207,13 @@ z-index: z("dropdown"); } -@media all and (min-width: 400px) { - #topic-progress, - #topic-progress-expanded { - right: 0; - left: 0; - } +#topic-progress, +#topic-progress-expanded { + right: 0; + left: 0; +} +@media all and (min-width: 400px) { #topic-footer-main-buttons { max-width: 70%; } diff --git a/app/assets/stylesheets/mobile/header.scss b/app/assets/stylesheets/mobile/header.scss index 720591b38ee..8ca1b8c49ee 100644 --- a/app/assets/stylesheets/mobile/header.scss +++ b/app/assets/stylesheets/mobile/header.scss @@ -84,7 +84,6 @@ } #main-outlet { - padding-top: 4.2857em; @media only screen and (orientation: landscape) { padding-right: env(safe-area-inset-right); padding-left: env(safe-area-inset-left);