diff --git a/js/src/forum/components/DiscussionPage.js b/js/src/forum/components/DiscussionPage.js index b168a68a0..6fc97119e 100644 --- a/js/src/forum/components/DiscussionPage.js +++ b/js/src/forum/components/DiscussionPage.js @@ -160,10 +160,8 @@ export default class DiscussionPage extends Page { * @param {Discussion} discussion */ show(discussion) { - this.discussion = discussion; - app.history.push('discussion', discussion.title()); - app.setTitle(this.discussion.title()); + app.setTitle(discussion.title()); app.setTitleCount(0); // When the API responds with a discussion, it will also include a number of @@ -194,10 +192,12 @@ export default class DiscussionPage extends Page { // posts we want to display. Tell the stream to scroll down and highlight // the specific post that was routed to. this.stream = new PostStreamState(discussion, includedPosts); - this.stream.goToNumber(m.route.param('near') || (includedPosts[0] && includedPosts[0].number()), true); + this.stream.goToNumber(m.route.param('near') || (includedPosts[0] && includedPosts[0].number()), true).then(() => { + this.discussion = discussion; - app.current.set('discussion', discussion); - app.current.set('stream', this.stream); + app.current.set('discussion', discussion); + app.current.set('stream', this.stream); + }); } /** diff --git a/js/src/forum/components/PostStream.js b/js/src/forum/components/PostStream.js index de3351845..37ee017ae 100644 --- a/js/src/forum/components/PostStream.js +++ b/js/src/forum/components/PostStream.js @@ -132,10 +132,10 @@ export default class PostStream extends Component { } if ('number' in newTarget) { - this.scrollToNumber(newTarget.number, this.stream.noAnimationScroll); + this.scrollToNumber(newTarget.number, this.stream.animateScroll); } else if ('index' in newTarget) { const backwards = newTarget.index === this.stream.count() - 1; - this.scrollToIndex(newTarget.index, this.stream.noAnimationScroll, backwards); + this.scrollToIndex(newTarget.index, this.stream.animateScroll, backwards); } this.prevTarget = newTarget; @@ -330,6 +330,7 @@ export default class PostStream extends Component { */ scrollToItem($item, animate, force, bottom) { const $container = $('html, body').stop(true); + const index = $item.data('index'); if ($item.length) { const itemTop = $item.offset().top - this.getMarginTop(); @@ -351,12 +352,32 @@ export default class PostStream extends Component { } } + // Even before scrolling, we want to provide location information + // to the scrubber as soon as possible. This way, we avoid an unnecessary + // and confusing animation. + this.updateScrubber(); + // We manually set the index because we want to display the index of the + // exact post we've scrolled to, not just that of the first post on the page. + this.stream.index = index; + this.stream.forceUpdateScrubber = true; + return Promise.all([$container.promise(), this.stream.loadPromise]).then(() => { - this.updateScrubber(); - const index = $item.data('index'); m.redraw.sync(); - const scroll = index == 0 ? 0 : $(`.PostStream-item[data-index=${$item.data('index')}]`).offset().top - this.getMarginTop(); - $(window).scrollTop(scroll); + + // After post data has been loaded in, we will attempt to scroll back + // to the top of the requested post (or to the top of the page if the + // first post was requested). In some cases, we may have scrolled to + // the end of the available post range, in which case, the next range + // of posts will be loaded in. However, in those cases, the post we + // requested won't exist, so scrolling to it would cause an error. + // Accordingly, we start by checking that it's offset is defined. + const offset = $(`.PostStream-item[data-index=${index}]`).offset(); + if (index === 0) { + $(window).scrollTop(0); + } else if (offset) { + $(window).scrollTop($(`.PostStream-item[data-index=${index}]`).offset().top - this.getMarginTop()); + } + this.calculatePosition(); this.stream.paused = false; }); diff --git a/js/src/forum/components/PostStreamScrubber.js b/js/src/forum/components/PostStreamScrubber.js index deb4cf2cf..d45474f05 100644 --- a/js/src/forum/components/PostStreamScrubber.js +++ b/js/src/forum/components/PostStreamScrubber.js @@ -90,7 +90,10 @@ export default class PostStreamScrubber extends Component { } onupdate() { - this.stream.loadPromise.then(() => this.updateScrubberValues({ animate: true, forceHeightChange: true })); + if (this.stream.forceUpdateScrubber) { + this.stream.forceUpdateScrubber = false; + this.stream.loadPromise.then(() => this.updateScrubberValues({ animate: true, forceHeightChange: true })); + } } oncreate(vnode) { @@ -137,7 +140,7 @@ export default class PostStreamScrubber extends Component { setTimeout(() => this.scrollListener.start()); - this.updateScrubberValues({ animate: true, forceHeightChange: true }); + this.stream.loadPromise.then(() => this.updateScrubberValues({ animate: false, forceHeightChange: true })); } onremove() { diff --git a/js/src/forum/states/PostStreamState.js b/js/src/forum/states/PostStreamState.js index 356bfa18d..277fd4959 100644 --- a/js/src/forum/states/PostStreamState.js +++ b/js/src/forum/states/PostStreamState.js @@ -37,6 +37,18 @@ class PostStreamState { */ this.description = ''; + /** + * When the page is scrolled, goToIndex is called, or the page is loaded, + * various listeners result in the scrubber being updated with a new + * position and values. However, if goToNumber is called, the scrubber + * will not be updated. Accordingly, we add logic to the scrubber's + * onupdate to update itself, but only when needed, as indicated by this + * property. + * + * @type {Boolean} + */ + this.forceUpdateScrubber = false; + this.show(includedPosts); } @@ -92,7 +104,7 @@ class PostStreamState { this.loadPromise = this.loadNearNumber(number); this.targetPost = { number }; - this.noAnimationScroll = noAnimation; + this.animateScroll = !noAnimation; this.number = number; // In this case, the redraw is only called after the response has been loaded @@ -116,7 +128,7 @@ class PostStreamState { this.loadPromise = this.loadNearIndex(index); this.targetPost = { index }; - this.noAnimationScroll = noAnimation; + this.animateScroll = !noAnimation; this.index = index; m.redraw();