From 5427b35c6dec2174d43588776fb2305c0c490cf2 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sun, 5 Jul 2020 19:04:09 -0400 Subject: [PATCH] Large simplifications of PostStreamScrubber --- js/src/forum/components/PostStream.js | 55 ++++ js/src/forum/components/PostStreamScrubber.js | 290 ++++-------------- js/src/forum/states/PostStreamState.js | 31 ++ 3 files changed, 139 insertions(+), 237 deletions(-) diff --git a/js/src/forum/components/PostStream.js b/js/src/forum/components/PostStream.js index 90da44a60..97a228e44 100644 --- a/js/src/forum/components/PostStream.js +++ b/js/src/forum/components/PostStream.js @@ -164,6 +164,61 @@ export default class PostStream extends Component { // viewport) to 100ms. clearTimeout(this.calculatePositionTimeout); this.calculatePositionTimeout = setTimeout(this.calculatePosition.bind(this), 100); + + // Before looping through all of the posts, we reset the scrollbar + // properties to a 'default' state. These values reflect what would be + // seen if the browser were scrolled right up to the top of the page, + // and the viewport had a height of 0. + const $items = $('.js-PostStream > .PostStream-item[data-index]'); + let index = $items.first().data('index') || 0; + let visible = 0; + let period = ''; + + // Now loop through each of the items in the discussion. An 'item' is + // either a single post or a 'gap' of one or more posts that haven't + // been loaded yet. + $items.each(function () { + const $this = $(this); + const top = $this.offset().top; + const height = $this.outerHeight(true); + + // If this item is above the top of the viewport, skip to the next + // one. If it's below the bottom of the viewport, break out of the + // loop. + if (top + height < viewportTop) { + return true; + } + if (top > viewportTop + viewportHeight) { + return false; + } + + // Work out how many pixels of this item are visible inside the viewport. + // Then add the proportion of this item's total height to the index. + const visibleTop = Math.max(0, viewportTop - top); + const visibleBottom = Math.min(height, viewportTop + viewportHeight - top); + const visiblePost = visibleBottom - visibleTop; + + if (top <= viewportTop) { + index = parseFloat($this.data('index')) + visibleTop / height; + } + + if (visiblePost > 0) { + visible += visiblePost / height; + } + + // If this item has a time associated with it, then set the + // scrollbar's current period to a formatted version of this time. + const time = $this.data('time'); + if (time) period = time; + }); + + const indexChanged = Math.floor(this.state.index) != Math.floor(index); + this.state.index = index; + this.state.visible = visible; + this.state.description = period ? dayjs(period).format('MMMM YYYY') : ''; + if (indexChanged) { + m.redraw(); + } } /** diff --git a/js/src/forum/components/PostStreamScrubber.js b/js/src/forum/components/PostStreamScrubber.js index 25f54b2b6..e27a32cf6 100644 --- a/js/src/forum/components/PostStreamScrubber.js +++ b/js/src/forum/components/PostStreamScrubber.js @@ -1,7 +1,5 @@ import Component from '../../common/Component'; import icon from '../../common/helpers/icon'; -import ScrollListener from '../../common/utils/ScrollListener'; -import SubtreeRetainer from '../../common/utils/SubtreeRetainer'; import formatNumber from '../../common/utils/formatNumber'; /** @@ -10,60 +8,24 @@ import formatNumber from '../../common/utils/formatNumber'; * * ### Props * - * - `discussion` * - `state` * - `className` */ export default class PostStreamScrubber extends Component { init() { - this.discussion = this.props.discusssion; this.state = this.props.state; this.handlers = {}; - - /** - * The index of the post that is currently at the top of the viewport. - * - * @type {Number} - */ - this.index = 0; - - /** - * The number of posts that are currently visible in the viewport. - * - * @type {Number} - */ - this.visible = 1; - - /** - * The description to render on the scrubber. - * - * @type {String} - */ - this.description = ''; - - // When the post stream begins loading posts at a certain index, we want our - // scrubber scrollbar to jump to that position. - this.state.on('unpaused', (this.handlers.streamWasUnpaused = this.streamWasUnpaused.bind(this))); - - // Define a handler to update the state of the scrollbar to reflect the - // current scroll position of the page. - this.scrollListener = new ScrollListener(this.onscroll.bind(this)); - - // Create a subtree retainer that will always cache the subtree after the - // initial draw. We render parts of the scrubber using this because we - // modify their DOM directly, and do not want Mithril messing around with - // our changes. - this.subtree = new SubtreeRetainer(() => true); } view() { - const retain = this.subtree.retain(); - const count = this.count(); - const unreadCount = this.discussion.unreadCount(); - const unreadPercent = count ? Math.min(count - this.index, unreadCount) / count : 0; + const index = this.state.index; + const count = this.state.count(); + const visible = this.state.visible || 1; + const unreadCount = this.state.discussion.unreadCount(); + const unreadPercent = count ? Math.min(count - this.state.index, unreadCount) / count : 0; const viewing = app.translator.transChoice('core.forum.post_scrubber.viewing_text', count, { - index: {retain || formatNumber(Math.min(Math.ceil(this.index + this.visible), count))}, + index: {formatNumber(Math.min(Math.ceil(index + visible), count))}, count: {formatNumber(count)}, }); @@ -83,8 +45,13 @@ export default class PostStreamScrubber extends Component { context.oldStyle = newStyle; } + const percentPerPost = this.percentPerPost(); + const beforeHeight = Math.max(0, percentPerPost.index * Math.min(index, count - visible)); + const handleHeight = Math.min(100 - beforeHeight, percentPerPost.visible * visible); + const afterHeight = 100 - beforeHeight - handleHeight; + return ( -
+
@@ -96,15 +63,15 @@ export default class PostStreamScrubber extends Component {
-
-
+
+
{viewing} - {retain || this.description} + {this.state.description}
-
+
{app.translator.trans('core.forum.post_scrubber.unread_text', { count: unreadCount })} @@ -120,13 +87,41 @@ export default class PostStreamScrubber extends Component { ); } + /** + * Get the percentage of the height of the scrubber that should be allocated + * to each post. + * + * @return {Object} + * @property {Number} index The percent per post for posts on either side of + * the visible part of the scrubber. + * @property {Number} visible The percent per post for the visible part of the + * scrubber. + */ + percentPerPost() { + const count = this.state.count() || 1; + const visible = this.state.visible || 1; + + // To stop the handle of the scrollbar from getting too small when there + // are many posts, we define a minimum percentage height for the handle + // calculated from a 50 pixel limit. From this, we can calculate the + // minimum percentage per visible post. If this is greater than the actual + // percentage per post, then we need to adjust the 'before' percentage to + // account for it. + const minPercentVisible = (50 / this.$('.Scrubber-scrollbar').outerHeight()) * 100; + const percentPerVisiblePost = Math.max(100 / count, minPercentVisible / visible); + const percentPerPost = count === visible ? 0 : (100 - percentPerVisiblePost * visible) / (count - visible); + + return { + index: percentPerPost, + visible: percentPerVisiblePost, + }; + } + /** * Go to the first post in the discussion. */ goToFirst() { this.state.goToFirst(); - this.index = 0; - this.renderScrollbar(true); } /** @@ -134,111 +129,6 @@ export default class PostStreamScrubber extends Component { */ goToLast() { this.state.goToLast(); - this.index = this.count(); - this.renderScrollbar(true); - } - - /** - * Get the number of posts in the discussion. - * - * @return {Integer} - */ - count() { - return this.state.count(); - } - - /** - * When the stream is unpaused, update the scrubber to reflect its position. - */ - streamWasUnpaused() { - this.update(window.pageYOffset); - this.renderScrollbar(true); - } - - /** - * Check whether or not the scrubber should be disabled, i.e. if all of the - * posts are visible in the viewport. - * - * @return {Boolean} - */ - disabled() { - return this.visible >= this.count(); - } - - /** - * When the page is scrolled, update the scrollbar to reflect the visible - * posts. - * - * @param {Integer} top - */ - onscroll(top) { - if (this.state.paused || !$('.js-PostStream')) return; - - this.update(top); - this.renderScrollbar(); - } - - /** - * Update the index/visible/description properties according to the window's - * current scroll position. - * - * @param {Integer} scrollTop - */ - update(scrollTop) { - const marginTop = $('#header').outerHeight() + parseInt($('.js-PostStream').css('margin-top'), 10); - const viewportTop = scrollTop + marginTop; - const viewportHeight = $(window).height() - marginTop; - - // Before looping through all of the posts, we reset the scrollbar - // properties to a 'default' state. These values reflect what would be - // seen if the browser were scrolled right up to the top of the page, - // and the viewport had a height of 0. - const $items = $('.js-PostStream > .PostStream-item[data-index]'); - let index = $items.first().data('index') || 0; - let visible = 0; - let period = ''; - - // Now loop through each of the items in the discussion. An 'item' is - // either a single post or a 'gap' of one or more posts that haven't - // been loaded yet. - $items.each(function () { - const $this = $(this); - const top = $this.offset().top; - const height = $this.outerHeight(true); - - // If this item is above the top of the viewport, skip to the next - // one. If it's below the bottom of the viewport, break out of the - // loop. - if (top + height < viewportTop) { - return true; - } - if (top > viewportTop + viewportHeight) { - return false; - } - - // Work out how many pixels of this item are visible inside the viewport. - // Then add the proportion of this item's total height to the index. - const visibleTop = Math.max(0, viewportTop - top); - const visibleBottom = Math.min(height, viewportTop + viewportHeight - top); - const visiblePost = visibleBottom - visibleTop; - - if (top <= viewportTop) { - index = parseFloat($this.data('index')) + visibleTop / height; - } - - if (visiblePost > 0) { - visible += visiblePost / height; - } - - // If this item has a time associated with it, then set the - // scrollbar's current period to a formatted version of this time. - const time = $this.data('time'); - if (time) period = time; - }); - - this.index = index; - this.visible = visible; - this.description = period ? dayjs(period).format('MMMM YYYY') : ''; } config(isInitialized, context) { @@ -246,8 +136,6 @@ export default class PostStreamScrubber extends Component { context.onunload = this.ondestroy.bind(this); - this.scrollListener.start(); - // Whenever the window is resized, adjust the height of the scrollbar // so that it fills the height of the sidebar. $(window) @@ -289,81 +177,12 @@ export default class PostStreamScrubber extends Component { } ondestroy() { - this.scrollListener.stop(); - - this.state.off('unpaused', this.handlers.streamWasUnpaused); - $(window).off('resize', this.handlers.onresize); $(document).off('mousemove touchmove', this.handlers.onmousemove).off('mouseup touchend', this.handlers.onmouseup); } - /** - * Update the scrollbar's position to reflect the current values of the - * index/visible properties. - * - * @param {Boolean} animate - */ - renderScrollbar(animate) { - const percentPerPost = this.percentPerPost(); - const index = this.index; - const count = this.count(); - const visible = this.visible || 1; - - const $scrubber = this.$(); - $scrubber.find('.Scrubber-index').text(formatNumber(Math.min(Math.ceil(index + visible), count))); - $scrubber.find('.Scrubber-description').text(this.description); - $scrubber.toggleClass('disabled', this.disabled()); - - const heights = {}; - heights.before = Math.max(0, percentPerPost.index * Math.min(index, count - visible)); - heights.handle = Math.min(100 - heights.before, percentPerPost.visible * visible); - heights.after = 100 - heights.before - heights.handle; - - const func = animate ? 'animate' : 'css'; - for (const part in heights) { - const $part = $scrubber.find(`.Scrubber-${part}`); - $part.stop(true, true)[func]({ height: heights[part] + '%' }, 'fast'); - - // jQuery likes to put overflow:hidden, but because the scrollbar handle - // has a negative margin-left, we need to override. - if (func === 'animate') $part.css('overflow', 'visible'); - } - } - - /** - * Get the percentage of the height of the scrubber that should be allocated - * to each post. - * - * @return {Object} - * @property {Number} index The percent per post for posts on either side of - * the visible part of the scrubber. - * @property {Number} visible The percent per post for the visible part of the - * scrubber. - */ - percentPerPost() { - const count = this.count() || 1; - const visible = this.visible || 1; - - // To stop the handle of the scrollbar from getting too small when there - // are many posts, we define a minimum percentage height for the handle - // calculated from a 50 pixel limit. From this, we can calculate the - // minimum percentage per visible post. If this is greater than the actual - // percentage per post, then we need to adjust the 'before' percentage to - // account for it. - const minPercentVisible = (50 / this.$('.Scrubber-scrollbar').outerHeight()) * 100; - const percentPerVisiblePost = Math.max(100 / count, minPercentVisible / visible); - const percentPerPost = count === visible ? 0 : (100 - percentPerVisiblePost * visible) / (count - visible); - - return { - index: percentPerPost, - visible: percentPerVisiblePost, - }; - } - onresize() { - this.scrollListener.update(); - // Adjust the height of the scrollbar so that it fills the height of // the sidebar and doesn't overlap the footer. const scrubber = this.$(); @@ -381,9 +200,8 @@ export default class PostStreamScrubber extends Component { onmousedown(e) { this.mouseStart = e.clientY || e.originalEvent.touches[0].clientY; - this.indexStart = this.index; + this.indexStart = this.state.index; this.dragging = true; - this.state.paused = true; $('body').css('cursor', 'move'); } @@ -397,10 +215,10 @@ export default class PostStreamScrubber extends Component { const deltaPixels = (e.clientY || e.originalEvent.touches[0].clientY) - this.mouseStart; const deltaPercent = (deltaPixels / this.$('.Scrubber-scrollbar').outerHeight()) * 100; const deltaIndex = deltaPercent / this.percentPerPost().index || 0; - const newIndex = Math.min(this.indexStart + deltaIndex, this.count() - 1); + const newIndex = Math.min(this.indexStart + deltaIndex, this.state.count() - 1); - this.index = Math.max(0, newIndex); - this.renderScrollbar(); + this.state.index = Math.max(0, newIndex); + m.redraw(); } onmouseup() { @@ -415,9 +233,8 @@ export default class PostStreamScrubber extends Component { // If the index we've landed on is in a gap, then tell the stream- // content that we want to load those posts. - const intIndex = Math.floor(this.index); + const intIndex = Math.floor(this.state.index); this.state.goToIndex(intIndex); - this.renderScrollbar(true); } onclick(e) { @@ -437,10 +254,9 @@ export default class PostStreamScrubber extends Component { // 3. Now we can convert the percentage into an index, and tell the stream- // content component to jump to that index. let offsetIndex = offsetPercent / this.percentPerPost().index; - offsetIndex = Math.max(0, Math.min(this.count() - 1, offsetIndex)); + offsetIndex = Math.max(0, Math.min(this.state.count() - 1, offsetIndex)); this.state.goToIndex(Math.floor(offsetIndex)); - this.index = offsetIndex; - this.renderScrollbar(true); + this.state.index = offsetIndex; this.$().removeClass('open'); } diff --git a/js/src/forum/states/PostStreamState.js b/js/src/forum/states/PostStreamState.js index 2dfb2a7b8..a7c0da4d6 100644 --- a/js/src/forum/states/PostStreamState.js +++ b/js/src/forum/states/PostStreamState.js @@ -21,6 +21,27 @@ class PostStreamState { this.loadPageTimeouts = {}; this.pagesLoading = 0; + /** + * The index of the post that is currently at the top of the viewport. + * + * @type {Number} + */ + this.index = 0; + + /** + * The number of posts that are currently visible in the viewport. + * + * @type {Number} + */ + this.visible = 1; + + /** + * The description to render on the scrubber. + * + * @type {String} + */ + this.description = ''; + this.show(includedPosts); } @@ -148,6 +169,16 @@ class PostStreamState { return this.discussion.postIds().length; } + /** + * Check whether or not the scrubber should be disabled, i.e. if all of the + * posts are visible in the viewport. + * + * @return {Boolean} + */ + allVisible() { + return this.visible >= this.count(); + } + /** * Are we currently viewing the end of the discussion? *