From 3e2231d2cb8860c731649f00ef85364bfcdc8ed1 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Sun, 27 Mar 2016 15:52:26 -0400 Subject: [PATCH] Fixing viewport shuffling due to image load Introduced new method ".loadImages()" in posts client side lib to handle viewport height changes when loading images. Requires nodebb-plugin-markdown@5.0.0 @BenLubar @boomzillawtf --- package.json | 2 +- public/less/global.less | 21 +++++++++++ public/src/client/topic.js | 13 ++++--- public/src/client/topic/posts.js | 60 ++++++++++++++++++++++++++++++++ public/src/modules/navigator.js | 25 +++++++++---- public/src/utils.js | 17 +++++++++ src/meta/css.js | 1 + 7 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 public/less/global.less diff --git a/package.json b/package.json index 016b488605..fab53b0d04 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "nodebb-plugin-composer-default": "3.0.18", "nodebb-plugin-dbsearch": "1.0.1", "nodebb-plugin-emoji-extended": "1.0.3", - "nodebb-plugin-markdown": "4.0.17", + "nodebb-plugin-markdown": "5.0.0", "nodebb-plugin-mentions": "1.0.21", "nodebb-plugin-soundpack-default": "0.1.6", "nodebb-plugin-spam-be-gone": "0.4.6", diff --git a/public/less/global.less b/public/less/global.less new file mode 100644 index 0000000000..80a5807ee3 --- /dev/null +++ b/public/less/global.less @@ -0,0 +1,21 @@ +/* + This stylesheet is applied to all themes and all pages. + They can be overridden by themes, though their presence (or initial settings) may be depended upon by + client-side logic in core. + + ========== +*/ + +/* Prevent viewport shuffling on image load by restricting image dimensions until viewed by the browser */ +[component="post/content"] img:not(.not-responsive) { + height: 1rem; + opacity: 0; + transition: width 500ms ease; + transition: height 500ms ease; + transition: opacity 500ms ease; + + &[data-state="loaded"] { + height: auto; + opacity: 1; + } +} \ No newline at end of file diff --git a/public/src/client/topic.js b/public/src/client/topic.js index 7a989f96aa..d5920d2118 100644 --- a/public/src/client/topic.js +++ b/public/src/client/topic.js @@ -61,12 +61,12 @@ define('forum/topic', [ addParentHandler(); - handleBookmark(tid); - handleKeys(); navigator.init('[component="post/anchor"]', ajaxify.data.postcount, Topic.toTop, Topic.toBottom, Topic.navigatorCallback, Topic.calculateIndex); + handleBookmark(tid); + $(window).on('scroll', updateTopicTitle); handleTopicSearch(); @@ -141,6 +141,7 @@ define('forum/topic', [ return navigator.scrollToPostIndex(postIndex, true); } } else if (bookmark && (!config.usePagination || (config.usePagination && ajaxify.data.pagination.currentPage === 1)) && ajaxify.data.postcount > 5) { + navigator.update(); app.alert({ alert_id: 'bookmark', message: '[[topic:bookmark_instructions]]', @@ -156,6 +157,8 @@ define('forum/topic', [ setTimeout(function() { app.removeAlert('bookmark'); }, 10000); + } else { + navigator.update(); } } @@ -233,7 +236,7 @@ define('forum/topic', [ return index; }; - Topic.navigatorCallback = function(index, elementCount) { + Topic.navigatorCallback = function(index, elementCount, threshold) { var path = ajaxify.removeRelativePath(window.location.pathname.slice(1)); if (!path.startsWith('topic')) { return 1; @@ -248,13 +251,13 @@ define('forum/topic', [ newUrl += '/' + index; } + posts.loadImages(threshold); + if (newUrl !== currentUrl) { if (Topic.replaceURLTimeout) { clearTimeout(Topic.replaceURLTimeout); } - Topic.replaceURLTimeout = setTimeout(function() { - updateUserBookmark(index); Topic.replaceURLTimeout = 0; diff --git a/public/src/client/topic/posts.js b/public/src/client/topic/posts.js index 1020edbf27..655277b704 100644 --- a/public/src/client/topic/posts.js +++ b/public/src/client/topic/posts.js @@ -231,6 +231,66 @@ define('forum/topic/posts', [ hidePostToolsForDeletedPosts(posts); }; + Posts.loadImages = function(threshold) { + /* + If threshold is defined, images loaded above this threshold will modify + the user's scroll position so they are not scrolled away from content + they were reading. Images loaded below this threshold will push down content. + + If no threshold is defined, loaded images will push down content, as per + default + */ + + var images = components.get('post/content').find('img[data-state="unloaded"]'), + visible = images.filter(function() { + return utils.isElementInViewport(this); + }), + scrollTop = $(window).scrollTop(), + adjusting = false, + adjustQueue = [], + adjustPosition = function() { + adjusting = true; + oldHeight = document.body.clientHeight; + + // Display the image + $(this).attr('data-state', 'loaded'); + newHeight = document.body.clientHeight; + + var imageRect = this.getBoundingClientRect(); + if (imageRect.top < threshold) { + scrollTop = scrollTop + (newHeight - oldHeight); + $(window).scrollTop(scrollTop); + } + + if (adjustQueue.length) { + adjustQueue.pop()(); + } else { + adjusting = false; + } + }, + oldHeight, newHeight; + + // For each image, reset the source and adjust scrollTop when loaded + visible.attr('data-state', 'loading'); + visible.each(function(index, image) { + image = $(image); + + image.on('load', function() { + if (!adjusting) { + adjustPosition.call(this); + } else { + adjustQueue.push(adjustPosition.bind(this)); + } + }); + + image.attr('src', image.attr('data-src')); + if (image.parent().attr('href')) { + image.parent().attr('href', image.attr('data-src')); + } + image.removeAttr('data-src'); + }); + }; + Posts.wrapImagesInLinks = function(posts) { posts.find('[component="post/content"] img:not(.emoji)').each(function() { var $this = $(this); diff --git a/public/src/modules/navigator.js b/public/src/modules/navigator.js index f6ac8c628a..2f6b2d8613 100644 --- a/public/src/modules/navigator.js +++ b/public/src/modules/navigator.js @@ -1,7 +1,7 @@ 'use strict'; -/* globals app, define, ajaxify, utils, config */ +/* globals define, ajaxify, utils, config */ define('navigator', ['forum/pagination', 'components'], function(pagination, components) { @@ -56,7 +56,6 @@ define('navigator', ['forum/pagination', 'components'], function(pagination, com }); navigator.setCount(count); - navigator.update(); }; function generateUrl(index) { @@ -92,7 +91,13 @@ define('navigator', ['forum/pagination', 'components'], function(pagination, com $('.pagination-block').toggleClass('ready', flag); } - navigator.update = function() { + navigator.update = function(threshold) { + threshold = typeof threshold === 'number' ? threshold : undefined; + + /* + The "threshold" is defined as the distance from the top of the page to + a spot where a user is expecting to begin reading. + */ var els = $(navigator.selector); if (els.length) { index = parseInt(els.first().attr('data-index'), 10) + 1; @@ -114,7 +119,7 @@ define('navigator', ['forum/pagination', 'components'], function(pagination, com }); if (typeof navigator.callback === 'function') { - navigator.callback(index, count); + navigator.callback(index, count, threshold); } navigator.updateTextAndProgressBar(); @@ -202,6 +207,9 @@ define('navigator', ['forum/pagination', 'components'], function(pagination, com return; } + // Temporarily disable navigator update on scroll + $(window).off('scroll', navigator.update); + duration = duration !== undefined ? duration : 400; navigator.scrollActive = true; var done = false; @@ -209,21 +217,24 @@ define('navigator', ['forum/pagination', 'components'], function(pagination, com function animateScroll() { var scrollTop = 0; if (postHeight < viewportHeight) { - scrollTop = (scrollTo.offset().top - (viewportHeight / 2) + (postHeight / 2)) + 'px'; + scrollTop = (scrollTo.offset().top - (viewportHeight / 2) + (postHeight / 2)); } else { scrollTop = scrollTo.offset().top - navbarHeight; } $('html, body').animate({ - scrollTop: scrollTop + scrollTop: scrollTop + 'px' }, duration, function() { if (done) { + // Re-enable onScroll behaviour + $(window).on('scroll', navigator.update); + var scrollToRect = scrollTo.get(0).getBoundingClientRect(); + navigator.update(scrollToRect.top); return; } done = true; navigator.scrollActive = false; - navigator.update(); highlightPost(); $('body').scrollTop($('body').scrollTop() - 1); $('html').scrollTop($('html').scrollTop() - 1); diff --git a/public/src/utils.js b/public/src/utils.js index 68de6a3506..78e0013c25 100644 --- a/public/src/utils.js +++ b/public/src/utils.js @@ -309,6 +309,23 @@ return labels; }, + /* Retrieved from http://stackoverflow.com/a/7557433 @ 27 Mar 2016 */ + isElementInViewport: function(el) { + //special bonus for those using jQuery + if (typeof jQuery === "function" && el instanceof jQuery) { + el = el[0]; + } + + var rect = el.getBoundingClientRect(); + + return ( + rect.top >= 0 && + rect.left >= 0 && + rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) && /*or $(window).height() */ + rect.right <= (window.innerWidth || document.documentElement.clientWidth) /*or $(window).width() */ + ); + }, + // get all the url params in a single key/value hash params: function(options) { var a, hash = {}, params; diff --git a/src/meta/css.js b/src/meta/css.js index 81fa0528c2..9dd9849d76 100644 --- a/src/meta/css.js +++ b/src/meta/css.js @@ -72,6 +72,7 @@ module.exports = function(Meta) { source += '\n@import "..' + path.sep + '..' + path.sep + 'public/less/blacklist.less";'; source += '\n@import "..' + path.sep + '..' + path.sep + 'public/less/generics.less";'; source += '\n@import "..' + path.sep + '..' + path.sep + 'public/less/mixins.less";'; + source += '\n@import "..' + path.sep + '..' + path.sep + 'public/less/global.less";'; var acpSource = '\n@import "..' + path.sep + 'public/less/admin/admin";\n' + source; acpSource += '\n@import "..' + path.sep + 'public/less/generics.less";';