Commit graph

9 commits

Author SHA1 Message Date
Jon Robson c06c547b02 Table of contents: FIXME cleanup
Follow up to Ib42d532d4e900c01061e1c5e39c03b17f0619c46

Hoist hook binding into main.js and update tests

main.js now only tests hook wire up.
tableOfContents calls reloadTableOfContents directly.

Bug: T316025
Change-Id: Ia06feab7cfb47a64ac3519bd7bd0428dc661045a
2023-02-24 01:16:20 +00:00
Jon Robson 450a395f95 SectionObserver should be updated at certain points in MediaWiki lifecycle
* When wikipage.tableOfContents is executed, the table of contents has been
replaced so we should update the sections associated with the section
observer as the old sections no longer exist in theDOM.
* If VisualEditor repaints the page, we should update the elements associated
with the table of contents as the body-content element no longer exists in the page.
* When VisuaLEditor is active we should disable the intersection observer as
there is no table of contents to update.

Minor changes:
* Light refactor of large function in main.js
* reloadTableOfContents returns a promise and now fires a new hook
wikipage.tableOfContents.vector to signal when it is finished which
has also been requested by community members. This allows the code in main.js
to update elements at relevant point in time
* Add FIXME note (doing this would have been considerable work)
* I've added a call to mw.log.warn which would have saved me considerable
debugging time for this issue to diagnose the issue.

Bug: T316037
Bug: T316025
Change-Id: Ib42d532d4e900c01061e1c5e39c03b17f0619c46
2023-02-23 01:39:46 +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 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
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
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