ScrollTo behavior does not take into account the Fixed elements setting if not on the frontpage, and also on multilingual websites.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff-3325962-4-7.txt | 1.25 KB | pivica |
| #7 | 3325962-7-fix-scrollto.patch | 1.66 KB | pivica |
ScrollTo behavior does not take into account the Fixed elements setting if not on the frontpage, and also on multilingual websites.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff-3325962-4-7.txt | 1.25 KB | pivica |
| #7 | 3325962-7-fix-scrollto.patch | 1.66 KB | pivica |
Comments
Comment #2
mathilde_dumond commentedComment #3
pivica commentedThx. Looks good, except comments should break on 80 line mark, but that is a minor thing.
Let's test this on live projects first before merging.
Comment #4
primsi commentedSwitching to the new once library and fixing the comment formatting mionor issue from #3.
Comment #5
primsi commentedComment #6
pivica commented> Switching to the new once library and fixing the comment formatting mionor issue from #3.
Great but if we are doing this then we should remove jQuery dependency for this JS file and related library, this is the only place we used jQuery $ object.
So let's remove jQuery from `js/anchor-scroll.js` and from library definition.
I also noticed we are missing `core/drupalSettings` dependency for `scroll_to_top` library, can we add it with the rest of this changes?
Comment #7
pivica commentedHere is a new patch that is removing jQuery fully from anchor-scroll.js and related lib.
> I also noticed we are missing `core/drupalSettings` dependency for `scroll_to_top` library
I mixed things here, this is not correct.
Comment #8
pivica commented@Primsi I am wondering why you decided to change to new once in the first place. Asking this, because if there is some additional reason then just removing depreciated dependency we should then also do this for scroll-to-top.js file. Quickly checked this file and we are using once and some usual jQuery methods so we can easily remove jQuery from this file too if/when needed in a follow up issue.
Comment #9
mathilde_dumond commentedtested that locally and the last patch fixed the problem
Comment #11
pivica commentedCommitted.