Given our use of constants for tracking classes this eslint rule
is more an annoyance than helpful.
Change-Id: I37570e3e851997d058f2d93777990dddb3d04089
Rather than test for fetch, limit the code to ES6 browsers.
Depends-On: I96a03796628a74ace93579d45a582711400c09c1
Change-Id: I4ca10182491118e61e155f99c713d4cb1b4fc7f0
We want the link that the user has clicked inside the TOC to be "active"
(e.g. bolded) regardless of whether the browser's scroll position
corresponds to that section. Therefore, we need to temporarily ignore
section observer until the browser has finished scrolling to the section
(if needed).
However, because the scroll event happens asyncronously after the user
clicks on a link and may not even happen at all (e.g. the user has
scrolled all the way to the bottom and clicks a section that is already
in the viewport), determining when we should resume section observer is
a bit tricky.
Because a scroll event may not even be triggered after clicking the
link, we instead allow the browser to perform a maximum number of
repaints before resuming sectionObserver. Per T297614#7687656, Firefox
wasn't consistently activating the table of contents section that the
user clicked even after waiting 2 frames. After further investigation,
it sometimes waits up to 3 frames before painting the new scroll
position so we have that as the limit.
Bug: T297614
Change-Id: If3632529f58c15348a7200258f4f5999ea0dadc4
This commits sets up the Table of Contents to bold the active section
when the section is scrolled.
Unfortunately, because our content does not have actual sections but
instead has a flat list of headings and paragraphs, we can't use
IntersectionObserver in the conventional way as it is optimized to find
intersections of elements that are *within* the viewport and the
callback will not reliably fire during certain scenarios (e.g. with fast
scrolling or when the headings are not currently within the viewport).
Furthermore, iterating through a list of elements and calling
`getBoundingClientRect()` can be expensive and can also cause
significant forced synchronous layouts that block the main thread.
The best compromise in terms of performance and function that I've found
is to use a combination of a throttled scroll event listener and
IntersectionObserver's ability to asyncronously find the
boundingClientRect of all elements off the main thread when `.observe`
is called which is the approach this patch takes. Although this is an
unorthodox way to use IntersectionObserver, performance profiles
recorded while holding the "down" arrow and scrolling for 10 seconds
with a 6x CPU throttle are comparable between master and this patch:
master: https://phabricator.wikimedia.org/F34930737
this patch: https://phabricator.wikimedia.org/F34930738
Bug: T297614
Change-Id: I4077d86a1786cc1f4a7d85b20b7cf402960940e7
Waiting for one animation frame seems to make the sticky header
re-appear consistently.
Bug: T299114
Change-Id: Ie1230bf861f12e4e18a6adb0f6779c199d6954a1
I haven't found any code responsible for making the scroll position
jump. It looks like Safari is doing this on its own. Looking at the
focus event in detail [1], it looks like there is an `preventScroll`
option you can pass to .focus() which might help in this situation, but
unfortunately, Safari doesn't seem to support this. Therfore, a hack
like this may be necessary.
[1] https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus
Bug: T297636
Change-Id: I90651293b7dd0f7f2970ba06255a12617b43661f
Move A/B test code to AB.js
Consolidate the show/hide code spread across scrollObserver
and stickyHeader by adding a show and hide function.
This is needed to fix T296680
Change-Id: Ia2e0c50278df0dfc1600610f281be20f4cc755c2
- Permits logging for scroll events without sticky header.
- Update function name to be more precise.
Bug: T292586
Change-Id: I441b4bf81bc4a36a03f0f1c215d86b01dce2911d
- Pull IntersectionObserver into new file to share observer with different callbacks:
- Wrap show/hide functionality of sticky header in conditionals based on user test group or by default.
- Fire hooks for scroll event tracking in WME.
- Add new js for A/B test functions and variables:
- Fire hook to send data for A/B test initialization.
- Update main js to include scrollObserver, A/B test init functionality.
- Add A/B test config.
- Update ResourceLoader package dependencies for sticky header.
- Though not a strict dependency, see I42e3e7c2084c1e88363d5d1662630ed23a28c4d2 in WME repo which uses these hooks to log scroll events.
- This patch includes changes from I56f40e706f8706fde1c0891a0561dd32c5e02bfc which were consolidated here for simplicity and ease of review - related to T292587 which calls for logging an init event for bucketing of users during A/B testing.
Bug: T292586
Change-Id: If6446e1e84cea3649905808c4f0e9f6862255fa3
stickyHeader.js, a file in the "skins.vector.es6" module, clones the
user menu. Because of this, it must initialize before dropdownMenu.js, a
file in the "skins.vector.js" module, in order for dropdownMenu.js to
bind the correct checkboxHack event listeners to the user menu in the
sticky header.
Therefore, change the es6 module to export its main method. The
skins.vector.js module can then use mw.loader.using to ensure the
skins.vector.es6 module initialization happens first in browsers that
support es6. Browsers that don't support es6 will continue to initialize
the skins.vector.js module.
Bug: T291096
Change-Id: I1bb6f2da9703ed2679eacfdb42b9818efe614ab9
Can be disabled via &vectorstickyheaderedit=0 or configuration
change.
This will allow us to fine tune the edit features without blocking
deploying the existing feature.
Bug: T294383
Change-Id: Ic282ea4f2ff0108eeaa154c8a77e4e5fd30daeae
Current expected behaviour: the editor experience will
load and the user will be thrown to the top of the page.
Bug: T293158
Change-Id: I3585616c2244a6b91ef5f160beb1cf51af3599aa
- Can now use const/let
- No need for feature detection for things like fetch and closest
as we can assume they exist if ES6 support is available
Change-Id: I85b01add13fd74e1514119498815403e42a09af0
This will allow us to write ES6 code for the new features which
is limited to those browsers.
For browsers that do not support ES6, the code will not execute
because of the "es6" flag. Doing this will help us avoid issues
like T293402
Change-Id: Iffb7098cb22395e33b87352fb4f08516f6f25e6f
bwang
jenkins-bot
Jon Robson
Nicholas Ray
Ed Sanders
Clare Ming