From 50feeed8bf10c96a38ded800395908b20e577b54 Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Tue, 24 Sep 2019 12:47:42 +0300 Subject: [PATCH 1/8] fix the bug with scrolling post comments into view --- webapp/components/CommentList/CommentList.vue | 27 +++++++++++++++++++ webapp/pages/post/_id.vue | 11 -------- webapp/pages/post/_id/_slug/index.vue | 2 +- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/webapp/components/CommentList/CommentList.vue b/webapp/components/CommentList/CommentList.vue index 17da8dda4..b5eb070c8 100644 --- a/webapp/components/CommentList/CommentList.vue +++ b/webapp/components/CommentList/CommentList.vue @@ -47,6 +47,33 @@ export default { return comment.id === updatedComment.id ? updatedComment : comment }) }, + scrollCommentsIntoView() { + if (!window || !document) { + return + } + const container = document.getElementById('comments') + if (container) { + const top = container.offsetTop + window.scroll({ + top, + left: 0, + behavior: 'smooth', + }) + } + }, + }, + watch: { + $route(to, from) { + // scroll inside the same page + if (to.hash === '#comments') { + this.scrollCommentsIntoView() + } + }, + }, + mounted() { + if (this.$route.hash === '#comments') { + setTimeout(this.scrollCommentsIntoView, 250) + } }, } diff --git a/webapp/pages/post/_id.vue b/webapp/pages/post/_id.vue index 3f8d93868..a02afd3b9 100644 --- a/webapp/pages/post/_id.vue +++ b/webapp/pages/post/_id.vue @@ -77,17 +77,6 @@ export default { ] }, }, - watch: { - $route(to, from) { - if (to.hash === '#comments') { - window.scroll({ - top: document.getElementById('comments').offsetTop, - left: 0, - behavior: 'smooth', - }) - } - }, - }, } diff --git a/webapp/pages/post/_id/_slug/index.vue b/webapp/pages/post/_id/_slug/index.vue index dd84ee3d7..147a04663 100644 --- a/webapp/pages/post/_id/_slug/index.vue +++ b/webapp/pages/post/_id/_slug/index.vue @@ -193,7 +193,7 @@ export default { .ds-card-image { img { - max-height: 300px; + height: 300px; object-fit: cover; object-position: center; } From e1751347fc59a5a1974389179e7e047ce89203bf Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Wed, 25 Sep 2019 10:32:35 +0300 Subject: [PATCH 2/8] fix failing client tests --- webapp/components/CommentList/CommentList.spec.js | 3 +++ webapp/components/FollowButton.vue | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/webapp/components/CommentList/CommentList.spec.js b/webapp/components/CommentList/CommentList.spec.js index 460f1a4ea..3e77d84fb 100644 --- a/webapp/components/CommentList/CommentList.spec.js +++ b/webapp/components/CommentList/CommentList.spec.js @@ -50,6 +50,9 @@ describe('CommentList.vue', () => { }, }, }, + $route: { + hash: '', + }, } stubs = { EditorContent: "
", diff --git a/webapp/components/FollowButton.vue b/webapp/components/FollowButton.vue index e1cad8c8e..22046e047 100644 --- a/webapp/components/FollowButton.vue +++ b/webapp/components/FollowButton.vue @@ -75,7 +75,7 @@ export default { const followedUser = follow ? data.followUser : data.unfollowUser this.$emit('update', followedUser) - } catch { + } catch (err) { optimisticResult.followedByCurrentUser = !follow this.$emit('optimistic', optimisticResult) } From b9c0749334cc70b43832d82320afa1d5395c3d75 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Sun, 29 Sep 2019 13:15:00 +0200 Subject: [PATCH 3/8] fix: scroll to top as default So nuxt's default scrollBehavior seems to have some kind of "scroll to anchor" already built in. Unfortunately we cannot use it, because the the anchor is not yet in the DOM when the scroll behavior is called. So the justification for this change is: 1. Remove scrollBehavior from `nuxt.config.js` it's deprecated to put it there. 2. Also *don't* use the default scrollbehavior because of the reasons above :point_up: Instead I assume to always scroll to the top. This might be undesired sometimes but let's keep a watchful eye and define the behavior if needed. --- webapp/app/router.scrollBehavior.js | 7 +++ webapp/nuxt.config.js | 74 ----------------------------- 2 files changed, 7 insertions(+), 74 deletions(-) create mode 100644 webapp/app/router.scrollBehavior.js diff --git a/webapp/app/router.scrollBehavior.js b/webapp/app/router.scrollBehavior.js new file mode 100644 index 000000000..041310f9c --- /dev/null +++ b/webapp/app/router.scrollBehavior.js @@ -0,0 +1,7 @@ +export default function(to, from, savedPosition) { + let position = { x: 0, y: 0 } + if (savedPosition) { + position = savedPosition + } + return position +} diff --git a/webapp/nuxt.config.js b/webapp/nuxt.config.js index 5e7726d43..f1747b479 100644 --- a/webapp/nuxt.config.js +++ b/webapp/nuxt.config.js @@ -124,80 +124,6 @@ export default { middleware: ['authenticated', 'termsAndConditions'], linkActiveClass: 'router-link-active', linkExactActiveClass: 'router-link-exact-active', - scrollBehavior: (to, _from, savedPosition) => { - let position = false - // if no children detected and scrollToTop is not explicitly disabled - if ( - to.matched.length < 2 && - to.matched.every(r => r.components.default.options.scrollToTop !== false) - ) { - // scroll to the top of the page - position = { - x: 0, - y: 0, - } - } else if (to.matched.some(r => r.components.default.options.scrollToTop)) { - // if one of the children has scrollToTop option set to true - position = { - x: 0, - y: 0, - } - } - - // savedPosition is only available for popstate navigations (back button) - if (savedPosition) { - position = savedPosition - } - - return new Promise(resolve => { - // wait for the out transition to complete (if necessary) - window.$nuxt.$once('triggerScroll', () => { - let processInterval = null - let processTime = 0 - const callInterval = 100 - const callIntervalLimit = 2000 - - // coords will be used if no selector is provided, - // or if the selector didn't match any element. - if (to.hash) { - let hash = to.hash - // CSS.escape() is not supported with IE and Edge. - if (typeof window.CSS !== 'undefined' && typeof window.CSS.escape !== 'undefined') { - hash = '#' + window.CSS.escape(hash.substr(1)) - } - try { - processInterval = setInterval(() => { - const hashIsFound = document.querySelector(hash) - - if (hashIsFound) { - position = { - selector: hash, - offset: { x: 0, y: -500 }, - } - } - processTime += callInterval - if (hashIsFound || processTime >= callIntervalLimit) { - clearInterval(processInterval) - processInterval = null - } - }, callInterval) - } catch (e) { - /* eslint-disable-next-line no-console */ - console.warn( - 'Failed to save scroll position. Please add CSS.escape() polyfill (https://github.com/mathiasbynens/CSS.escape).', - ) - } - } - - let resolveInterval = setInterval(() => { - if (!processInterval) { - clearInterval(resolveInterval) - resolve(position) - } - }, callInterval) - }) - }) - }, }, /* From 57598df22899c178e41b1068cb3e9cd22723d69c Mon Sep 17 00:00:00 2001 From: roschaefer Date: Sun, 29 Sep 2019 13:34:27 +0200 Subject: [PATCH 4/8] refactor: re-use @vbelolapotkov's solution If we make this a mixin, we can re-use the same solution for e.g. the comment. If sb. notifies you, the browser automatically scrolls to the comment in which you have been mentioned. --- webapp/components/Comment.vue | 2 ++ .../CommentList/CommentList.spec.js | 3 -- webapp/components/CommentList/CommentList.vue | 29 ++--------------- webapp/mixins/scrollToAnchor.js | 32 +++++++++++++++++++ 4 files changed, 36 insertions(+), 30 deletions(-) create mode 100644 webapp/mixins/scrollToAnchor.js diff --git a/webapp/components/Comment.vue b/webapp/components/Comment.vue index 2cebde0b6..b1a75f896 100644 --- a/webapp/components/Comment.vue +++ b/webapp/components/Comment.vue @@ -79,8 +79,10 @@ import ContentMenu from '~/components/ContentMenu' import ContentViewer from '~/components/Editor/ContentViewer' import HcCommentForm from '~/components/CommentForm/CommentForm' import CommentMutations from '~/graphql/CommentMutations' +import scrollToAnchor from '~/mixins/scrollToAnchor.js' export default { + mixins: [scrollToAnchor], data: function() { return { isCollapsed: true, diff --git a/webapp/components/CommentList/CommentList.spec.js b/webapp/components/CommentList/CommentList.spec.js index 3e77d84fb..460f1a4ea 100644 --- a/webapp/components/CommentList/CommentList.spec.js +++ b/webapp/components/CommentList/CommentList.spec.js @@ -50,9 +50,6 @@ describe('CommentList.vue', () => { }, }, }, - $route: { - hash: '', - }, } stubs = { EditorContent: "
", diff --git a/webapp/components/CommentList/CommentList.vue b/webapp/components/CommentList/CommentList.vue index b5eb070c8..2013140c1 100644 --- a/webapp/components/CommentList/CommentList.vue +++ b/webapp/components/CommentList/CommentList.vue @@ -32,8 +32,10 @@ diff --git a/webapp/mixins/scrollToAnchor.js b/webapp/mixins/scrollToAnchor.js new file mode 100644 index 000000000..e68658f07 --- /dev/null +++ b/webapp/mixins/scrollToAnchor.js @@ -0,0 +1,32 @@ +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) + } +} + +export default { + watch: { + $route(to, from) { + const anchor = to && to.hash + scrollToAnchor(anchor) + }, + }, + mounted() { + const anchor = this.$route && this.$route.hash + scrollToAnchor(anchor) + }, +} From 9da40c4895342459aa9e0942358b06c8559f8205 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 1 Oct 2019 11:55:18 +0200 Subject: [PATCH 5/8] 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" From db1bcdd3d2a0dbf77e66bbf6a17e531bcce3b3b9 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 1 Oct 2019 17:25:28 +0200 Subject: [PATCH 6/8] refactor: register vue-scrollto in nuxt.config.js This will allow us to use this.$scrollTo in components. I'm now also using this in the mixin. With so many `this`s it gets horribly difficult to properly test the mixin in isolation. So I decided to test the mixin on the component directly. --- webapp/components/Comment/Comment.spec.js | 33 ++++++++++++- .../CommentList/CommentList.spec.js | 37 ++++++++++++++- webapp/mixins/scrollToAnchor.js | 21 +++++---- webapp/mixins/scrollToAnchor.spec.js | 47 ------------------- webapp/nuxt.config.js | 1 + 5 files changed, 81 insertions(+), 58 deletions(-) delete mode 100644 webapp/mixins/scrollToAnchor.spec.js diff --git a/webapp/components/Comment/Comment.spec.js b/webapp/components/Comment/Comment.spec.js index 381d49bc2..9dc16777e 100644 --- a/webapp/components/Comment/Comment.spec.js +++ b/webapp/components/Comment/Comment.spec.js @@ -32,6 +32,7 @@ describe('Comment.vue', () => { truncate: a => a, removeHtml: a => a, }, + $scrollTo: jest.fn(), $apollo: { mutate: jest.fn().mockResolvedValue({ data: { @@ -51,6 +52,8 @@ describe('Comment.vue', () => { }) describe('shallowMount', () => { + beforeEach(jest.useFakeTimers) + Wrapper = () => { const store = new Vuex.Store({ getters, @@ -117,7 +120,35 @@ describe('Comment.vue', () => { }) }) - beforeEach(jest.useFakeTimers) + describe('scrollToAnchor mixin', () => { + describe('$route.hash !== comment.id', () => { + beforeEach(() => { + mocks.$route = { + hash: '', + } + }) + + it('skips $scrollTo', () => { + wrapper = Wrapper() + jest.runAllTimers() + expect(mocks.$scrollTo).not.toHaveBeenCalled() + }) + }) + + describe('$route.hash === comment.id', () => { + beforeEach(() => { + mocks.$route = { + hash: '#commentId-2', + } + }) + + it('calls $scrollTo', () => { + wrapper = Wrapper() + jest.runAllTimers() + expect(mocks.$scrollTo).toHaveBeenCalledWith('#commentId-2', 1000) + }) + }) + }) describe('test callbacks', () => { beforeEach(() => { diff --git a/webapp/components/CommentList/CommentList.spec.js b/webapp/components/CommentList/CommentList.spec.js index 4d382b36d..0eacf8628 100644 --- a/webapp/components/CommentList/CommentList.spec.js +++ b/webapp/components/CommentList/CommentList.spec.js @@ -42,6 +42,7 @@ describe('CommentList.vue', () => { truncate: a => a, removeHtml: a => a, }, + $scrollTo: jest.fn(), $apollo: { queries: { Post: { @@ -65,12 +66,46 @@ describe('CommentList.vue', () => { }) } - beforeEach(() => { + it('displays a comments counter', () => { wrapper = Wrapper() + expect(wrapper.find('span.ds-tag').text()).toEqual('1') }) it('displays a comments counter', () => { + wrapper = Wrapper() expect(wrapper.find('span.ds-tag').text()).toEqual('1') }) + + describe('scrollToAnchor mixin', () => { + beforeEach(jest.useFakeTimers) + + describe('$route.hash !== `#comments`', () => { + beforeEach(() => { + mocks.$route = { + hash: '', + } + }) + + it('skips $scrollTo', () => { + wrapper = Wrapper() + jest.runAllTimers() + expect(mocks.$scrollTo).not.toHaveBeenCalled() + }) + }) + + describe('$route.hash === `#comments`', () => { + beforeEach(() => { + mocks.$route = { + hash: '#comments', + } + }) + + it('calls $scrollTo', () => { + wrapper = Wrapper() + jest.runAllTimers() + expect(mocks.$scrollTo).toHaveBeenCalledWith('#comments', 1000) + }) + }) + }) }) }) diff --git a/webapp/mixins/scrollToAnchor.js b/webapp/mixins/scrollToAnchor.js index 3ce4eb5a9..70a1bf695 100644 --- a/webapp/mixins/scrollToAnchor.js +++ b/webapp/mixins/scrollToAnchor.js @@ -1,20 +1,23 @@ -import { scrollTo } from 'vue-scrollto' +function scrollToAnchor(anchor, { checkAnchor, $scrollTo }) { + if (typeof checkAnchor !== 'function') + throw new Error( + 'You must define `checkAnchor` on the component if you use scrollToAnchor mixin!', + ) + if (!checkAnchor(anchor)) return + setTimeout(() => { + $scrollTo(anchor, 1000) + }, 250) +} export default { watch: { $route(to, from) { const anchor = to && to.hash - if (!this.checkAnchor(anchor)) return - setTimeout(() => { - scrollTo(anchor, 1000) - }, 250) + scrollToAnchor(anchor, this) }, }, mounted() { const anchor = this.$route && this.$route.hash - if (!this.checkAnchor(anchor)) return - setTimeout(() => { - scrollTo(anchor, 1000) - }, 250) + scrollToAnchor(anchor, this) }, } diff --git a/webapp/mixins/scrollToAnchor.spec.js b/webapp/mixins/scrollToAnchor.spec.js deleted file mode 100644 index a9447e458..000000000 --- a/webapp/mixins/scrollToAnchor.spec.js +++ /dev/null @@ -1,47 +0,0 @@ -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/nuxt.config.js b/webapp/nuxt.config.js index f1747b479..4867ad5d6 100644 --- a/webapp/nuxt.config.js +++ b/webapp/nuxt.config.js @@ -142,6 +142,7 @@ export default { keys: envWhitelist, }, ], + 'vue-scrollto/nuxt', 'cookie-universal-nuxt', '@nuxtjs/apollo', '@nuxtjs/axios', From 08e73747bd440f19d0dd8e3cfa5baaa22f9fd6c0 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 1 Oct 2019 17:29:49 +0200 Subject: [PATCH 7/8] refactor: set global defaults of vue-scrollto --- webapp/components/Comment/Comment.spec.js | 2 +- webapp/components/CommentList/CommentList.spec.js | 2 +- webapp/mixins/scrollToAnchor.js | 2 +- webapp/nuxt.config.js | 8 +++++++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/webapp/components/Comment/Comment.spec.js b/webapp/components/Comment/Comment.spec.js index 9dc16777e..59e3f6c5a 100644 --- a/webapp/components/Comment/Comment.spec.js +++ b/webapp/components/Comment/Comment.spec.js @@ -145,7 +145,7 @@ describe('Comment.vue', () => { it('calls $scrollTo', () => { wrapper = Wrapper() jest.runAllTimers() - expect(mocks.$scrollTo).toHaveBeenCalledWith('#commentId-2', 1000) + expect(mocks.$scrollTo).toHaveBeenCalledWith('#commentId-2') }) }) }) diff --git a/webapp/components/CommentList/CommentList.spec.js b/webapp/components/CommentList/CommentList.spec.js index 0eacf8628..3287b7cd4 100644 --- a/webapp/components/CommentList/CommentList.spec.js +++ b/webapp/components/CommentList/CommentList.spec.js @@ -103,7 +103,7 @@ describe('CommentList.vue', () => { it('calls $scrollTo', () => { wrapper = Wrapper() jest.runAllTimers() - expect(mocks.$scrollTo).toHaveBeenCalledWith('#comments', 1000) + expect(mocks.$scrollTo).toHaveBeenCalledWith('#comments') }) }) }) diff --git a/webapp/mixins/scrollToAnchor.js b/webapp/mixins/scrollToAnchor.js index 70a1bf695..2966c79ed 100644 --- a/webapp/mixins/scrollToAnchor.js +++ b/webapp/mixins/scrollToAnchor.js @@ -5,7 +5,7 @@ function scrollToAnchor(anchor, { checkAnchor, $scrollTo }) { ) if (!checkAnchor(anchor)) return setTimeout(() => { - $scrollTo(anchor, 1000) + $scrollTo(anchor) }, 250) } diff --git a/webapp/nuxt.config.js b/webapp/nuxt.config.js index 4867ad5d6..84602f7eb 100644 --- a/webapp/nuxt.config.js +++ b/webapp/nuxt.config.js @@ -142,7 +142,13 @@ export default { keys: envWhitelist, }, ], - 'vue-scrollto/nuxt', + [ + 'vue-scrollto/nuxt', + { + offset: -100, // to compensate fixed navbar height + duration: 1000, + }, + ], 'cookie-universal-nuxt', '@nuxtjs/apollo', '@nuxtjs/axios', From 24b2cab473985effdc4692d261f986dea54d6105 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 1 Oct 2019 17:43:38 +0200 Subject: [PATCH 8/8] fix: handle edge case noticed by @vbelolapotkov --- webapp/app/router.scrollBehavior.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/webapp/app/router.scrollBehavior.js b/webapp/app/router.scrollBehavior.js index 041310f9c..d34c4c38a 100644 --- a/webapp/app/router.scrollBehavior.js +++ b/webapp/app/router.scrollBehavior.js @@ -1,7 +1,10 @@ export default function(to, from, savedPosition) { - let position = { x: 0, y: 0 } - if (savedPosition) { - position = savedPosition - } - return position + if (savedPosition) return savedPosition + + // Edge case: If you click on a notification from a comment and then on the + // post page you click on 'comments', we avoid a "jumping" scroll behavior, + // ie. jump to the top and scroll back from there + if (to.path === from.path && to.hash !== from.hash) return false + + return { x: 0, y: 0 } }