Commit graph

39 commits

Author SHA1 Message Date
jenkins-bot 11b968cf18 Merge "Clean up: Remove ts-ignores in stickyheader.js" 2022-02-23 16:18:06 +00:00
jenkins-bot d93f9e9bed Merge "Use TOC template data for showing collapsible section arrows" 2022-02-22 22:16:18 +00:00
bwang 5f0c0cb294 Use TOC template data for showing collapsible section arrows
Bug: T299361
Depends-on: I8ab5c0543b898d1df9399a1cb39672c45daf2acd
Change-Id: Ib68de8cd97cc1111a5a33e100e688d6832fc7e6e
2022-02-22 15:47:23 -06:00
Nicholas Ray 27939ac6c9 Revert "Improve jsdoc for tableOfContents.js and sectionObserver.js"
This mostly reverts commit f5ad6fe78a but
keeps the SectionObserverProps typedef.

Instead, we will use @module introduced in
Ib68de8cd97cc1111a5a33e100e688d6832fc7e6e.

Change-Id: I7aff49a3d922889cc99bc4313a6cb416410a7a0d
2022-02-22 12:36:35 -07:00
Nicholas Ray f5ad6fe78a Improve jsdoc for tableOfContents.js and sectionObserver.js
Switch to using @namespace which seems to have the best jsdoc support
for the revealing module pattern [1] that these two files use.

Additionally:

* Add typedef for section observer props

[1] https://www.oreilly.com/library/view/learning-javascript-design/9781449334840/ch09s03.html

Change-Id: I3eeda191e5da0294ad40533053adb57e1fc9c8e9
2022-02-21 17:40:10 -07:00
Nicholas Ray 6e9506dcad Dynamically expand/collapse sub-sections in ToC based on # of headings
Server render the table of contents in a collapsed state when the total
number of headings is equal or greater than the value of
`$wgVectorTableOfContentsCollapseAtCount`. Otherwise, the table of
contents will be server rendered in its "expanded" state.

In addition:

* Revise table of contents tests to call one `assertion` per element so
  that it is easier to see the exact element that may fail an assertion.
* Revise table of contents tests to call a mount function that can merge
  props to allow for a more flexible set of tests.
* Revise table of contents tests by wrapping a `describe` around tests
  that expect the same prop state.
* Adds typedef for table of sections props

Bug: T300973
Depends-On: Ifaee451e1903f2accd0ada2f2ed6dfa3f83037b6
Change-Id: I382200bc603b6abf757a91f14a8a55a6581969bd
2022-02-21 14:58:51 -07:00
bwang 48f1f1355b Clean up: Remove ts-ignores in stickyheader.js
From what I could tell, most instances of `toHTMLElement` or @ts-ignore could be fixed by using typeguards or using Element over HTMLElement.
Element is a looser typing than HTMLElement, but given `querySelector` returns `Element|null` and the fact that we were already inconsistently using both Element and HTMLElement in this file, I feel it was a worthwhile tradeoff

Change-Id: I3512a98fa67c13a4383b9497e8588960259b5b68
2022-02-21 11:41:12 -06:00
Jon Robson 2dcd99ed21 Sticky header dropdown should not contain gadgets in personal menu
Bug: T302087
Change-Id: Iae1f42c3c526398d7dea038786ae9001e118307b
2022-02-18 21:30:29 +00:00
bwang 301e09916d Toggle ToC sections when clicking toggle button
Bug: T300167
Change-Id: If1150a9e018b232da900187383aaee9c9cf331a1
2022-02-16 15:48:28 -06:00
jenkins-bot 0f49a257d1 Merge "Collapse ToC by default & expand sections when clicking section headings" 2022-02-15 19:31:18 +00:00
Jan Drewniak 7d32ec80d3 Collapse ToC by default & expand sections when clicking section headings
Collapses sub-sections in the new table of contents by default
(except for non-js and reduced-motion users) and expands the
sections when the top-level section link has been clicked.

Refactors the `activateSection` TableOfContents methods into separate
`activateSection` and `deactivateSection` functions.
Adds `expandSection` and `collapseSection` methods.

Adds triangle icon as a visual expand/collapsed indicator
next to all ToC section headings and are hidden via CSS based on
whether or not the section contains subsections.

Adds test for tableOfContents.

Bug: T299361
Change-Id: I36b3ae7f9f633877683bc17a9444c970d7fa7293
2022-02-15 00:09:50 -05:00
jenkins-bot 4557c0910b Merge "Use new ve.activationStart hook to hide header earlier when loading editor" 2022-02-14 18:33:19 +00:00
jenkins-bot a541859743 Merge "[eslint] Disable mediawiki/class-doc" 2022-02-14 16:15:34 +00:00
Nicholas Ray 6b73bd2b41 Fix jsdoc comments for sectionObserver.js
Move jsdoc comment closer to the methods they are describing. This also
enables better typehint support.

I36b3ae7f9f633877683bc17a9444c970d7fa7293 will handle revising tableOfContents.js.

Change-Id: Ifcac7cfd88cd3f1c0405611c880a0d101d2aed3b
2022-02-11 12:17:38 -07:00
bwang 52c7c2ee75 [eslint] Disable mediawiki/class-doc
Given our use of constants for tracking classes this eslint rule
is more an annoyance than helpful.

Change-Id: I37570e3e851997d058f2d93777990dddb3d04089
2022-02-11 16:31:33 +00:00
jenkins-bot 306dc89bf8 Merge "Limit WVUI search to ES6 browsers" 2022-02-09 23:01:03 +00:00
Jon Robson 0c2981d772 Limit WVUI search to ES6 browsers
Rather than test for fetch, limit the code to ES6 browsers.

Depends-On: I96a03796628a74ace93579d45a582711400c09c1
Change-Id: I4ca10182491118e61e155f99c713d4cb1b4fc7f0
2022-02-09 22:10:11 +00:00
Nicholas Ray 80a111d0e4 Fix TOC section activation on link click bug
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
2022-02-08 14:45:16 -07:00
Nicholas Ray 4d2ad52374 Remove getElementsByClassName usage from sectionObserver
Based on prior discussion [1], using getElementsByClassName probably
isn't worth it.

[1] https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/756675/8..13/resources/skins.vector.es6/main.js#104

Change-Id: Id7f8aff73a1d2082ebfeaa9488a815d96317c156
2022-02-03 17:33:42 -07:00
bwang 66359e8fa5 Setup jest unit tests and add basic test cases for AB.js and App.vue
Bug: T300561
Change-Id: Ib7c314b094bd823ae233374f63c9094724d6c06f
2022-01-31 20:50:33 +00:00
Ed Sanders 1cd5bf4d41 Use new ve.activationStart hook to hide header earlier when loading editor
Bug: T299907
Depends-On: I0eaeb98719bf7a43e4a87366cfcd204f35b74650
Change-Id: I8e8b635b46e79c63c4dafbd2418c9be94528ec06
2022-01-31 18:32:55 +00:00
Nicholas Ray 3c433a5315 Add sectionObserver and tableOfContents component JS to respond to intersection changes
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
2022-01-26 14:11:43 -07:00
Ed Sanders da1fb74554 Sticky header: Wait for some repainting to happen after VE teardown
Waiting for one animation frame seems to make the sticky header
re-appear consistently.

Bug: T299114
Change-Id: Ie1230bf861f12e4e18a6adb0f6779c199d6954a1
2022-01-14 14:41:36 +00:00
Nicholas Ray 17e742e2ab Reset scroll position when sticky header search input receives focus to fix Safari bug
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
2022-01-06 15:30:21 -07:00
bwang bd799ecc2e Add watchstar to sticky header (alternative)
Bug: T294759
Depends-on: I88af8585e8fc75f77ebef867d267199aeb2c6592
Change-Id: I15c409830ef8970ff7319b4dd447904443949b8d
2021-12-21 09:40:48 -08:00
jdlrobson d79dbf37c9 Don't use ts-ignore. It is hiding real errors
We are passing the wrong arguments to
addVisualEditorHooks

Bug: T297119
Change-Id: I2f8ced4513a1e5fcca2a2a2393cbb8fd7a3db388
2021-12-16 10:27:42 +00:00
Clare Ming 60553ff324 Prevent A/B test enrollment hook from firing for unsampled
Bug: T297662
Change-Id: Ibeca32a3c0fab7de403d69ea274c653cf4bd6c0e
2021-12-13 17:31:17 -07:00
jenkins-bot cc44e46229 Merge "Define sticky header ULS behaviour" 2021-12-09 23:15:05 +00:00
jdlrobson 51cce2d0d4 Define sticky header ULS behaviour
* It sticks to the header on scroll
* It hides when the sticky header hides

Bug: T296680
Change-Id: I5a4e2ba42e172ea55fbdac8f35ec895f6b2756cd
2021-12-09 22:18:35 +00:00
Clare Ming 454a2845b4 Update A/B test enrollment name
Bug: T292587
Change-Id: Ib4174119e18496139bb942032a2401ebc4d1849f
2021-12-09 13:16:54 -07:00
jdlrobson d834329e15 [Sticky header refactor] Separate responsibilities
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
2021-12-03 14:30:10 -07:00
Clare Ming 37c9a24f75 Update scroll observer to allow event logging
- Permits logging for scroll events without sticky header.
- Update function name to be more precise.

Bug: T292586
Change-Id: I441b4bf81bc4a36a03f0f1c215d86b01dce2911d
2021-12-01 16:12:06 -07:00
jenkins-bot e95e48ce51 Merge "Add scroll event + init A/B test logging to sticky header, AB js" 2021-11-09 23:26:17 +00:00
Clare Ming 457dcfc472 Add scroll event + init A/B test logging to sticky header, AB js
- 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
2021-11-09 15:00:25 -07:00
Nicholas Ray c741759cab Initialize the skins.vector.es6 module before the skins.vector.js module
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
2021-11-03 11:03:18 -06:00
jdlrobson 768a07ec6c Add sticky header edit feature flag
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
2021-10-26 21:59:29 +00:00
jdlrobson 0663087dd5 Sticky header edit icons trigger via JavaScript
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
2021-10-25 22:35:13 +00:00
jdlrobson ca0401789d ES6-ify sticky header code
- 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
2021-10-21 23:44:30 +00:00
jdlrobson b8122cc40b Separate code from ES6 browsers from ES5 code
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
2021-10-21 15:55:04 -07:00