jenkins-bot has submitted this change and it was merged.
Change subject: Also scroll the TOC when it is toggled
......................................................................
Also scroll the TOC when it is toggled
Broke scrollTocToActiveItem into its own method, so it
can be called multiple times and be easily tested.
Boost number of TOC titles fetched per batch to 50
Also, add a class name for the TOC <ul>
Bug: T78572
Bug: T85944
Change-Id: Ica03a7b07cf8d63a9f73b208aa474b5b6354e219
---
M handlebars/compiled/flow_block_topiclist.handlebars.php
M handlebars/flow_board_navigation.handlebars
M modules/engine/components/board/features/flow-board-navigation.js
M modules/engine/components/board/features/flow-board-toc.js
4 files changed, 66 insertions(+), 38 deletions(-)
Approvals:
Mattflaschen: Looks good to me, approved
EBernhardson: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/handlebars/compiled/flow_block_topiclist.handlebars.php
b/handlebars/compiled/flow_block_topiclist.handlebars.php
index 331631b..4e6a365 100644
--- a/handlebars/compiled/flow_block_topiclist.handlebars.php
+++ b/handlebars/compiled/flow_block_topiclist.handlebars.php
@@ -65,7 +65,7 @@
data-flow-load-handler="menu"
data-flow-toc-target=".flow-list">
<div class="flow-menu-js-drop
flow-menu-js-drop-hidden"><a href="javascript:void(0);"
class="flow-board-header-menu-activator"></a></div>
- <ul class="mw-ui-button-container flow-list
flow-load-interactive"
+ <ul class="mw-ui-button-container flow-board-toc-list
flow-list flow-load-interactive"
data-flow-load-handler="tocMenu"
data-flow-toc-target="li:not(.flow-load-more):last"
data-flow-template="flow_board_toc_loop">
@@ -468,4 +468,4 @@
</div>
';
}
-?>
\ No newline at end of file
+?>
diff --git a/handlebars/flow_board_navigation.handlebars
b/handlebars/flow_board_navigation.handlebars
index b6d0e29..94b4c66 100644
--- a/handlebars/flow_board_navigation.handlebars
+++ b/handlebars/flow_board_navigation.handlebars
@@ -42,7 +42,7 @@
data-flow-load-handler="menu"
data-flow-toc-target=".flow-list">
<div class="flow-menu-js-drop
flow-menu-js-drop-hidden"><a href="javascript:void(0);"
class="flow-board-header-menu-activator"></a></div>
- <ul class="mw-ui-button-container flow-list
flow-load-interactive"
+ <ul class="mw-ui-button-container flow-board-toc-list
flow-list flow-load-interactive"
data-flow-load-handler="tocMenu"
data-flow-toc-target="li:not(.flow-load-more):last"
data-flow-template="flow_board_toc_loop">
diff --git a/modules/engine/components/board/features/flow-board-navigation.js
b/modules/engine/components/board/features/flow-board-navigation.js
index 577f9db..793e380 100644
--- a/modules/engine/components/board/features/flow-board-navigation.js
+++ b/modules/engine/components/board/features/flow-board-navigation.js
@@ -151,15 +151,20 @@
};
}
+ // TODO: Let's look at decoupling the event handler part from the parts
that actually do the
+ // work. (Already, event is not used.)
/**
* On window.scroll, we clone the nav header bar and fix the original
to the window top.
* We clone so that we have one which always remains in the same place
for calculation purposes,
* as it can vary depending on whether or not new content is rendered
or the window is resized.
- * @param {jQuery} $boardNavigation
- * @param {Event} event
+ * @param {jQuery} $boardNavigation board navigation element
+ * @param {Event} event Event passed to windowScroll (unused)
+ * @param {Object} extraParameters
+ * @param {boolean} extraParameters.forceNavigationUpdate True to force
a change to the
+ * active item and TOC scroll.
*/
function _flowBoardAdjustTopicNavigationHeader( $boardNavigation,
event, extraParameters ) {
- var bottomScrollPosition, topicText, newReadingTopicId,
$tocContainer, $scrollTarget,
+ var bottomScrollPosition, topicText, newReadingTopicId,
self = this,
boardNavigationPosition = ( this.$boardNavigationClone
|| $boardNavigation ).offset();
@@ -266,37 +271,7 @@
return;
}
- // Set TOC active item
- $tocContainer = this.$boardNavigationTitle.parent()
- .findWithParent(
this.$boardNavigationTitle.parent().data( 'flow-api-target' ) );
-
- $scrollTarget = $tocContainer.find( 'a[data-flow-id]' )
- .removeClass( 'active' )
- .filter( '[data-flow-id=' + this.readingTopicId + ']' )
- .addClass( 'active' )
- .closest( 'li' )
- .next();
-
- if ( !$scrollTarget.length ) {
- // we are at the last list item; use the current one
instead
- $scrollTarget = $scrollTarget.end();
- }
- // Scroll to the active item
- if ( $scrollTarget.length ) {
- $tocContainer.scrollTop( $scrollTarget.offset().top -
$tocContainer.offset().top + $tocContainer.scrollTop() );
- // the above may not trigger the scroll.flow-load-more
event within the TOC if the $tocContainer
- // does not have a scrollbar. If that happens you could
have a TOC without a scrollbar
- // that refuses to autoload anything else. Fire it
again(wasteful) untill we find
- // a better way.
- // This does not seem to work for the initial load,
that is handled in flow-boad-loadmore.js
- // when it runs this same code. This seems to be
required for subsequent loads after
- // the initial call.
- if ( this.$loadMoreNodes ) {
- this.$loadMoreNodes
- .filter(
'[data-flow-api-handler=topicList]' )
- .trigger( 'scroll.flow-load-more', {
forceNavigationUpdate: true } );
- }
- }
+ this.scrollTocToActiveItem();
}
// Mixin to FlowComponent
diff --git a/modules/engine/components/board/features/flow-board-toc.js
b/modules/engine/components/board/features/flow-board-toc.js
index c5b9d5c..a5c0c4a 100644
--- a/modules/engine/components/board/features/flow-board-toc.js
+++ b/modules/engine/components/board/features/flow-board-toc.js
@@ -67,7 +67,11 @@
isLoadMoreButton = $this.data( 'flow-load-handler' )
=== 'loadMore';
if ( !isLoadMoreButton && !( extraParameters || {}
).skipMenuToggle ) {
- // Also open the menu on this node
+ // Re-scroll the TOC (in case the scroll that tracks
the page scroll failed
+ // due to insufficient elements making the desired
scrollTop not work (T78572)).
+ info.component.scrollTocToActiveItem();
+
+ // Actually open/close the TOC menu on this node.
$this.trigger( 'click', { interactiveHandler:
'menuToggle' } );
}
@@ -80,6 +84,7 @@
// Send some overrides to this API request
var overrides = {
topiclist_sortby: info.component.$board.data(
'flow-sortby' ),
+ topiclist_limit: 50,
topiclist_toconly: true
};
@@ -225,6 +230,8 @@
// On element-load handlers
//
+ // This is a confusing name since this.$tocMenu is set to
flow-board-toc-list, whereas you
+ // would expect flow-board-toc-menu.
/**
* Stores the TOC menu for later use.
* @param {jQuery} $tocMenu
@@ -284,6 +291,52 @@
FlowBoardComponentTocFeatureMixin.UI.events.loadHandlers.topicTitle =
flowBoardComponentTocFeatureElementLoadTopicTitle;
//
+ // Public functions
+ //
+
+ /**
+ * Scroll the TOC to the active item
+ */
+ function flowBoardComponentTocFeatureScrollTocToActiveItem() {
+ // Set TOC active item
+ var $tocContainer = this.$tocMenu,
+ requestedScrollTop, afterScrollTop; // For debugging
+
+ var $scrollTarget = $tocContainer.find( 'a[data-flow-id]' )
+ .removeClass( 'active' )
+ .filter( '[data-flow-id=' + this.readingTopicId + ']' )
+ .addClass( 'active' )
+ .closest( 'li' )
+ .next();
+
+ if ( !$scrollTarget.length ) {
+ // we are at the last list item; use the current one
instead
+ $scrollTarget = $scrollTarget.end();
+ }
+ // Scroll to the active item
+ if ( $scrollTarget.length ) {
+ requestedScrollTop = $scrollTarget.offset().top -
$tocContainer.offset().top + $tocContainer.scrollTop();
+ $tocContainer.scrollTop( requestedScrollTop );
+ afterScrollTop = $tocContainer.scrollTop();
+ // the above may not trigger the scroll.flow-load-more
event within the TOC if the $tocContainer
+ // does not have a scrollbar. If that happens you could
have a TOC without a scrollbar
+ // that refuses to autoload anything else. Fire it
again(wasteful) untill we find
+ // a better way.
+ // This does not seem to work for the initial load,
that is handled in flow-boad-loadmore.js
+ // when it runs this same code. This seems to be
required for subsequent loads after
+ // the initial call.
+ if ( this.$loadMoreNodes ) {
+ this.$loadMoreNodes
+ .filter(
'[data-flow-api-handler=topicList]' )
+ .trigger( 'scroll.flow-load-more', {
forceNavigationUpdate: true } );
+ }
+ }
+
+ }
+
+ FlowBoardComponentTocFeatureMixin.prototype.scrollTocToActiveItem =
flowBoardComponentTocFeatureScrollTocToActiveItem;
+
+ //
// Private functions
//
--
To view, visit https://gerrit.wikimedia.org/r/183386
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ica03a7b07cf8d63a9f73b208aa474b5b6354e219
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <mflasc...@wikimedia.org>
Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Mattflaschen <mflasc...@wikimedia.org>
Gerrit-Reviewer: SG <shah...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
