From 15323b3d63f4398414279654bd58e8ebd7cd8ddf Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Thu, 29 Aug 2019 17:02:35 -0700 Subject: [PATCH] Use core table of contents in Minerva No more using the TableOfContents component in MobileFrontend. It's just creating more work for us. The end result is exactly the same - we can make a table of contents using the checkbox hack rule and CSS that looks identical to the current table of contents. For now, this change can only be tested on Minerva desktop. I2ea1c23bc86871e2a095c4c6674a08ff2f04b160 is the patch that goes for the jugular and applies this to mobile Minerva. It's important we merge the two together to avoid disruption to this feature, as currently MobileFrontend strips the table of contents from core from the HTML using the MobileFormatter. Change-Id: I720e62a578f0c7a14f4b5a698004471c85e54bc8 --- .../skins.minerva.base.styles/common.less | 8 -- resources/skins.minerva.base.styles/toc.less | 0 .../skins.minerva.content.styles/main.less | 2 + .../tablet/common.less | 20 ----- .../skins.minerva.content.styles/toc.less | 78 +++++++++++++++++++ resources/skins.minerva.scripts/init.js | 22 +++--- resources/skins.minerva.scripts/styles.less | 23 ++++++ resources/skins.minerva.scripts/toc.js | 41 ---------- skin.json | 3 +- .../features/support/pages/article_page.rb | 2 +- 10 files changed, 118 insertions(+), 81 deletions(-) create mode 100644 resources/skins.minerva.base.styles/toc.less create mode 100644 resources/skins.minerva.content.styles/toc.less delete mode 100644 resources/skins.minerva.scripts/toc.js diff --git a/resources/skins.minerva.base.styles/common.less b/resources/skins.minerva.base.styles/common.less index 5a3f049dd..77cd34edf 100644 --- a/resources/skins.minerva.base.styles/common.less +++ b/resources/skins.minerva.base.styles/common.less @@ -7,14 +7,6 @@ .box-sizing( border-box ); } -/* Hide the table of contents `.toc-mobile` unless the user is viewing in tablet resolution or - * higher */ -.toc-mobile, -/* Table of contents `.toc` as provided by parser has no styling, this is a temporary measure - * until we are able to commit more time to Minerva on desktop */ -.toc, -/* We also need a more specific rule for tablet non-JS users who will load skins.minerva.tablet.styles */ -.client-nojs .toc-mobile, .client-js .mw-redirectedfrom, /* FIXME: Use generic rule for print stylesheets */ .printfooter, diff --git a/resources/skins.minerva.base.styles/toc.less b/resources/skins.minerva.base.styles/toc.less new file mode 100644 index 000000000..e69de29bb diff --git a/resources/skins.minerva.content.styles/main.less b/resources/skins.minerva.content.styles/main.less index 28fcf50bf..e7217a365 100644 --- a/resources/skins.minerva.content.styles/main.less +++ b/resources/skins.minerva.content.styles/main.less @@ -1,6 +1,8 @@ @import '../../minerva.less/minerva.variables.less'; @import '../../minerva.less/minerva.mixins.less'; +@import 'toc.less'; + // Content formatting and typography // // Our content is predominately text, hence visual hierarchy must be clear. diff --git a/resources/skins.minerva.content.styles/tablet/common.less b/resources/skins.minerva.content.styles/tablet/common.less index 623be3a68..32ddbac2a 100644 --- a/resources/skins.minerva.content.styles/tablet/common.less +++ b/resources/skins.minerva.content.styles/tablet/common.less @@ -4,7 +4,6 @@ A file for css that optimises the Minerva skin on larger devices. @import '../../../minerva.less/minerva.variables.less'; @import '../../../minerva.less/minerva.mixins.less'; -@paddingVertical: 1.4em; @media screen and ( min-width: @width-breakpoint-tablet ) { .client-js { @@ -28,25 +27,6 @@ A file for css that optimises the Minerva skin on larger devices. } } - .toc-mobile { - // Reset the rule for mobile mode (but not for .client-nojs) - display: table; - visibility: visible; - position: relative; - font-size: 1.3em; - margin: 1em 0; - border: solid 1px transparent; - - > h2 { - visibility: hidden; - font-family: @fontFamilyBase; - font-size: @tocFontSize; - font-weight: bold; - border-bottom: 0; - padding: @paddingVertical / 2 0; - } - } - // FIXME: Have a class that identifies all of these selectors .pre-content, /* Form only pages e.g. Special:MobileOptions */ diff --git a/resources/skins.minerva.content.styles/toc.less b/resources/skins.minerva.content.styles/toc.less new file mode 100644 index 000000000..64680cba2 --- /dev/null +++ b/resources/skins.minerva.content.styles/toc.less @@ -0,0 +1,78 @@ +@import '../../minerva.less/minerva.variables.less'; + +// FIXME: While we transition to the new table of contents hide the old mobile one +// which would appear unstyled on mobile. +// This can be removed when cache has cleared for patch +// I720e62a578f0c7a14f4b5a698004471c85e54bc8 +#toc.toc-mobile { + display: none; +} + +#toc { + display: none; + position: relative; + margin: 1em 0; + background-color: @colorGray15; + border: solid 1px @grayLightest; + box-sizing: border-box; +} + +.toctogglecheckbox ~ ul { + display: none; +} + +.toctogglecheckbox:checked ~ ul { + display: block; +} + +.toctogglelabel { + cursor: pointer; + position: absolute; + left: 0; + top: 0; + right: 0; + height: 50px; + z-index: @z-indexAboveContent; +} + +.client-js .content .toc { + .toctitle { + @toctitle-vertical-padding: 1.4em / 2; + visibility: hidden; + background-position: right center; + font-weight: bold; + border-bottom: 0; + padding: @toctitle-vertical-padding 3.5em @toctitle-vertical-padding 3.5em; + + h2 { + font-family: @fontFamilyBase; + font-size: unit( 18/16, em ); + } + } + + .tocnumber { + display: none; + } + + > ul { + font-size: @font-size-minerva-small; + margin-left: 52px; + margin-right: 24px; + padding: 0 0 20px; + } + + ul { + list-style: none; + } +} + +@media screen and ( min-width: @width-breakpoint-tablet ) { + #toc { + // Reset the rule for mobile mode (but not for .client-nojs) + display: table; + + .toctitle { + visibility: visible; + } + } +} diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index f432818e8..9c33a4402 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -3,8 +3,8 @@ mobile = M.require( 'mobile.startup' ), PageGateway = mobile.PageGateway, toast = mobile.toast, + Icon = mobile.Icon, time = mobile.time, - toc = require( './toc.js' ), errorLogging = require( './errorLogging.js' ), notifications = require( './notifications.js' ), preInit = require( './preInit.js' ), @@ -304,7 +304,7 @@ } $( function () { - var $toc, + var toolbarElement = document.querySelector( Toolbar.selector ), userMenu = document.querySelector( '.minerva-user-menu' ); // See UserMenuDirector. // Init: @@ -342,16 +342,20 @@ notifications(); } - // add a ToC only for "view" action (user is reading a page) - // provided a table of contents placeholder has been rendered - // eslint-disable-next-line no-jquery/no-global-selector - $toc = $( '#toc' ); - if ( mw.config.get( 'wgAction' ) === 'view' && $toc.length > 0 ) { - toc( currentPage, $toc ); - } mw.requestIdleCallback( errorLogging ); // deprecation notices mw.log.deprecate( router, 'navigate', router.navigate, 'use navigateTo instead' ); + + // setup toc icons + new Icon( { + glyphPrefix: 'mf', + name: 'toc' + } ).$el.prependTo( '.toctitle' ); + new Icon( { + glyphPrefix: 'mf', + name: 'arrow', + isSmall: true + } ).$el.appendTo( '.toctitle' ); } ); module.exports = { overlayManager: overlayManager diff --git a/resources/skins.minerva.scripts/styles.less b/resources/skins.minerva.scripts/styles.less index 14b0a4627..9a3c3955d 100644 --- a/resources/skins.minerva.scripts/styles.less +++ b/resources/skins.minerva.scripts/styles.less @@ -4,6 +4,29 @@ @animationDuration: 0.3s; +// Table of contents icons +.toctitle { + .mw-ui-icon { + position: absolute; + top: 1.2em; + + &:first-child { + left: 0; + } + + &:last-child { + right: 0; + } + } +} + +// flip the arrow in table of contents when toggled +.toctogglecheckbox:checked ~ .toctitle .mw-ui-icon:last-child { + &:before { + transform: rotate( -180deg ); + } +} + // Last modified bar styles .last-modified-bar { &.active { diff --git a/resources/skins.minerva.scripts/toc.js b/resources/skins.minerva.scripts/toc.js deleted file mode 100644 index 50826a68d..000000000 --- a/resources/skins.minerva.scripts/toc.js +++ /dev/null @@ -1,41 +0,0 @@ -( function ( M ) { - var mobile = M.require( 'mobile.startup' ), - Toggler = mobile.Toggler, - TableOfContents = mobile.toc.TableOfContents, - eventBus = mobile.eventBusSingleton; - - /** - * Create TableOfContents if the given Page has sections and is not the main page - * and wgMFTocEnabled config variable is set to true. - * @method - * @param {Page} page for which a TOC is generated - * @param {jQuery.Object} $toc container to replace - * @ignore - */ - function init( page, $toc ) { - var sections = page.getSections(), - toc = new TableOfContents( { - sections: sections - } ); - - // eslint-disable-next-line no-new - new Toggler( { - $container: toc.$el, - prefix: 'toc-', - page: page, - isClosed: true, - eventBus: eventBus - } ); - // if there is a toc already, replace it - if ( $toc.length > 0 ) { - // don't show toc at end of page, when no sections there - $toc.replaceWith( toc.$el ); - } else { - // otherwise append it to the lead section - toc.appendTo( page.getLeadSectionElement() ); - } - } - - module.exports = init; - -}( mw.mobileFrontend ) ); diff --git a/skin.json b/skin.json index f8b52e53e..72fbfc2f1 100644 --- a/skin.json +++ b/skin.json @@ -580,8 +580,7 @@ "resources/skins.minerva.scripts/Toolbar.js", "resources/skins.minerva.scripts/mobileRedirect.js", "resources/skins.minerva.scripts/search.js", - "resources/skins.minerva.scripts/references.js", - "resources/skins.minerva.scripts/toc.js" + "resources/skins.minerva.scripts/references.js" ] }, "skins.minerva.options.share.icon": { diff --git a/tests/browser/features/support/pages/article_page.rb b/tests/browser/features/support/pages/article_page.rb index 815541ebf..925b1e172 100644 --- a/tests/browser/features/support/pages/article_page.rb +++ b/tests/browser/features/support/pages/article_page.rb @@ -98,7 +98,7 @@ class ArticlePage # rubocop:disable Metrics/ClassLength div(:wikidata_description, css: '.tagline') # toc - div(:toc, css: '.toc-mobile') + div(:toc, css: '.toc') # editor (common) span(:overlay_editor_mode_switcher, css: '.editor-switcher .oo-ui-indicatorElement-indicator')