From 9da40c4895342459aa9e0942358b06c8559f8205 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 1 Oct 2019 11:55:18 +0200 Subject: [PATCH] fix: avoid many scrollTo calls for n components Thank you @vbelolapotkov for pointing out the flaws here: https://github.com/Human-Connection/Human-Connection/pull/1756#discussion_r329361572 So here is my attempt to fix it: * Install `vue-scrollto` which relies on `requestAnimationFrame` - apparently this is better on Safari and IE? :thinking: - Mocking out entire modules is easier in jest: https://jestjs.io/docs/en/bypassing-module-mocks * Require `checkAnchor` to be implemented on the component --- webapp/components/Comment/Comment.vue | 8 +++- webapp/components/CommentList/CommentList.vue | 3 ++ webapp/mixins/scrollToAnchor.js | 30 ++++-------- webapp/mixins/scrollToAnchor.spec.js | 47 +++++++++++++++++++ webapp/package.json | 1 + webapp/yarn.lock | 12 +++++ 6 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 webapp/mixins/scrollToAnchor.spec.js diff --git a/webapp/components/Comment/Comment.vue b/webapp/components/Comment/Comment.vue index 1c80e4caf..31bbc811f 100644 --- a/webapp/components/Comment/Comment.vue +++ b/webapp/components/Comment/Comment.vue @@ -10,7 +10,7 @@
- + @@ -111,6 +111,9 @@ export default { user: 'auth/user', isModerator: 'auth/isModerator', }), + anchor() { + return `commentId-${this.comment.id}` + }, displaysComment() { return !this.unavailable || this.isModerator }, @@ -144,6 +147,9 @@ export default { }, }, methods: { + checkAnchor(anchor) { + return `#${this.anchor}` === anchor + }, isAuthor(id) { return this.user.id === id }, diff --git a/webapp/components/CommentList/CommentList.vue b/webapp/components/CommentList/CommentList.vue index d1374ec42..f1cdf910b 100644 --- a/webapp/components/CommentList/CommentList.vue +++ b/webapp/components/CommentList/CommentList.vue @@ -41,6 +41,9 @@ export default { post: { type: Object, default: () => {} }, }, methods: { + checkAnchor(anchor) { + return anchor === '#comments' + }, updateCommentList(updatedComment) { this.post.comments = this.post.comments.map(comment => { return comment.id === updatedComment.id ? updatedComment : comment diff --git a/webapp/mixins/scrollToAnchor.js b/webapp/mixins/scrollToAnchor.js index e68658f07..3ce4eb5a9 100644 --- a/webapp/mixins/scrollToAnchor.js +++ b/webapp/mixins/scrollToAnchor.js @@ -1,32 +1,20 @@ -export function scrollToAnchor(anchor) { - if (!anchor) return - if (!window || !document) { - return - } - const container = document.querySelector(anchor) - if (container) { - const { top } = container.getBoundingClientRect() - setTimeout(() => { - // we have to set a small timeout to ensure this part comes after nuxt - // scrollBehaviour: https://nuxtjs.org/api/configuration-router/#scrollbehavior - window.scroll({ - top, - left: 0, - behavior: 'smooth', - }) - }, 250) - } -} +import { scrollTo } from 'vue-scrollto' export default { watch: { $route(to, from) { const anchor = to && to.hash - scrollToAnchor(anchor) + if (!this.checkAnchor(anchor)) return + setTimeout(() => { + scrollTo(anchor, 1000) + }, 250) }, }, mounted() { const anchor = this.$route && this.$route.hash - scrollToAnchor(anchor) + if (!this.checkAnchor(anchor)) return + setTimeout(() => { + scrollTo(anchor, 1000) + }, 250) }, } diff --git a/webapp/mixins/scrollToAnchor.spec.js b/webapp/mixins/scrollToAnchor.spec.js new file mode 100644 index 000000000..a9447e458 --- /dev/null +++ b/webapp/mixins/scrollToAnchor.spec.js @@ -0,0 +1,47 @@ +import { scrollTo } from 'vue-scrollto' +import scrollToAnchor from './scrollToAnchor' +jest.mock('vue-scrollto') + +let component + +describe('scrollToAnchor', () => { + beforeEach(() => { + jest.useFakeTimers() + scrollTo.mockClear() + }) + + describe('scrollToAnchor', () => { + const action = hash => { + let { + watch: { $route }, + } = scrollToAnchor + $route.bind(component)({ hash }) + jest.runAllTimers() + } + + describe('given anchor `commentId-4711`', () => { + beforeEach(() => { + component = { + anchor: 'commentId-4711', + checkAnchor(anchor) { + return this.anchor === anchor + }, + } + }) + + describe('$route.hash === anchor', () => { + it('calls window.scroll', () => { + action('commentId-4711') + expect(scrollTo).toHaveBeenCalled() + }) + }) + + describe('$route.hash !== anchor', () => { + it('skips window.scroll', () => { + action('commentId-4712') + expect(scrollTo).not.toHaveBeenCalled() + }) + }) + }) + }) +}) diff --git a/webapp/package.json b/webapp/package.json index 841b32c53..89e64ebde 100644 --- a/webapp/package.json +++ b/webapp/package.json @@ -83,6 +83,7 @@ "vue-count-to": "~1.0.13", "vue-infinite-scroll": "^2.0.2", "vue-izitoast": "^1.2.1", + "vue-scrollto": "^2.17.1", "vue-sweetalert-icons": "~4.2.0", "vuex-i18n": "~1.13.1", "xregexp": "^4.2.4", diff --git a/webapp/yarn.lock b/webapp/yarn.lock index 49375e89c..2298def4a 100644 --- a/webapp/yarn.lock +++ b/webapp/yarn.lock @@ -4235,6 +4235,11 @@ bcrypt-pbkdf@^1.0.0: dependencies: tweetnacl "^0.14.3" +bezier-easing@2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/bezier-easing/-/bezier-easing-2.1.0.tgz#c04dfe8b926d6ecaca1813d69ff179b7c2025d86" + integrity sha1-wE3+i5JtbsrKGBPWn/F5t8ICXYY= + bfj@^6.1.1: version "6.1.1" resolved "https://registry.yarnpkg.com/bfj/-/bfj-6.1.1.tgz#05a3b7784fbd72cfa3c22e56002ef99336516c48" @@ -15315,6 +15320,13 @@ vue-router@~3.0.7: resolved "https://registry.yarnpkg.com/vue-router/-/vue-router-3.0.7.tgz#b36ca107b4acb8ff5bc4ff824584059c23fcb87b" integrity sha512-utJ+QR3YlIC/6x6xq17UMXeAfxEvXA0VKD3PiSio7hBOZNusA1jXcbxZxVEfJunLp48oonjTepY8ORoIlRx/EQ== +vue-scrollto@^2.17.1: + version "2.17.1" + resolved "https://registry.yarnpkg.com/vue-scrollto/-/vue-scrollto-2.17.1.tgz#cd62ee0b98cf7e2ba9fd94f029addcd093978a48" + integrity sha512-uxOJXg6cZL88B+hTXRHDJMR+gHGiaS70ZTNk55fE5Z2TdwyIx9K/IHoNeTrtBrM6u3FASAIymKjZaQLmDf8Ykg== + dependencies: + bezier-easing "2.1.0" + vue-server-renderer@^2.6.10: version "2.6.10" resolved "https://registry.yarnpkg.com/vue-server-renderer/-/vue-server-renderer-2.6.10.tgz#cb2558842ead360ae2ec1f3719b75564a805b375"