jenkins-bot has submitted this change and it was merged. Change subject: Collapse sections by default ......................................................................
Collapse sections by default A global function mfTempOpenSection is added to the top of the page and all heading elements have click handlers bound to it. As soon as skins.minerva.toggling is enabled these will be unbound and the global destroyed and it will be business as usual. In the mean time for eager readers it will be possible to expand sections. It will not be possible to collapse them again (easily added but not worth the additional complexity) The experience remains the same as before for non-js users thanks to client-nojs and toggle.js will honour any existing sections that are open Bug: T126825 Change-Id: Ia342b3ddfbf0a419122aa0de282310396fa264e7 --- M includes/MobileFormatter.php M includes/MobileFrontend.hooks.php M includes/MobileFrontend.skin.hooks.php M includes/api/ApiParseExtender.php M resources/mobile.toggle/toggle.less M resources/skins.minerva.content.styles/main.less M resources/skins.minerva.toggling/init.js M tests/phpunit/MobileFormatterTest.php 8 files changed, 93 insertions(+), 19 deletions(-) Approvals: Jhobs: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/MobileFormatter.php b/includes/MobileFormatter.php index 75fe6bf..13b6f3f 100644 --- a/includes/MobileFormatter.php +++ b/includes/MobileFormatter.php @@ -10,6 +10,12 @@ */ class MobileFormatter extends HtmlFormatter { /** + * Whether scripts can be added in the output. + * @var boolean $scriptsEnabled + */ + private $scriptsEnabled = true; + + /** * The current revision id of the Title being worked on * @var Integer $revId */ @@ -63,6 +69,12 @@ ->getMFConfig()->get( 'MFMobileFormatterHeadings' ); } + /** + * Disables the generation of script tags in output HTML. + */ + public function disableScripts() { + $this->scriptsEnabled = false; + } /** * Creates and returns a MobileFormatter * @@ -421,9 +433,8 @@ if ( $this->mainPage ) { $element = $this->parseMainPage( $this->getDoc() ); } - $html = parent::getText( $element ); - return $html; + return parent::getText( $element ); } /** @@ -533,16 +544,23 @@ $sectionNumber = 0; $sectionBody = $this->createSectionBodyElement( $doc, $sectionNumber ); - // Mark the top level headings which will become collapsible soon. + // Mark the top level headings which could control collapsing foreach ( $headings as $heading ) { + $sectionNumber += 1; $className = $heading->hasAttribute( 'class' ) ? $heading->getAttribute( 'class' ) . ' ' : ''; $heading->setAttribute( 'class', $className . 'section-heading' ); + if ( $this->scriptsEnabled ) { + $heading->setAttribute( 'onclick', 'javascript:mfTempOpenSection(' . $sectionNumber . ')' ); + } + // prepend indicator $indicator = $doc->createElement( 'div' ); $indicator->setAttribute( 'class', MobileUI::iconClass( '', 'element', 'indicator' ) ); $heading->insertBefore( $indicator, $heading->firstChild ); } + // reset section number + $sectionNumber = 0; while ( $sibling ) { $node = $sibling; $sibling = $sibling->nextSibling; @@ -606,10 +624,17 @@ * @return DOMElement */ private function createSectionBodyElement( DOMDocument $doc, $sectionNumber ) { + $sectionClass = 'mf-section-' . $sectionNumber; + if ( $sectionNumber > 0 && $this->scriptsEnabled ) { + // TODO: Probably good to rename this to the more generic 'section'. + // We have no idea how the skin will use this. + $sectionClass .= ' collapsible-block'; + } + // FIXME: The class `/mf\-section\-[0-9]+/` is kept for caching reasons // but given class is unique usage is discouraged. [T126825] $sectionBody = $doc->createElement( 'div' ); - $sectionBody->setAttribute( 'class', 'mf-section-' . $sectionNumber ); + $sectionBody->setAttribute( 'class', $sectionClass ); $sectionBody->setAttribute( 'id', 'mf-section-' . $sectionNumber ); return $sectionBody; } diff --git a/includes/MobileFrontend.hooks.php b/includes/MobileFrontend.hooks.php index e5f2943..1d752a4 100644 --- a/includes/MobileFrontend.hooks.php +++ b/includes/MobileFrontend.hooks.php @@ -177,6 +177,7 @@ * * Applies MobileFormatter to mobile viewed content * Also enables Related Articles in the footer in the beta mode. + * Adds inline script to allow opening of sections while JS is still loading * * @param OutputPage $out * @param string $text the HTML to be wrapped inside the #mw-content-text element @@ -186,6 +187,8 @@ global $wgRelatedArticlesFooterBlacklistedSkins; $context = MobileContext::singleton(); + $title = $context->getTitle(); + // Perform a few extra changes if we are in mobile mode if ( $context->shouldDisplayMobileView() ) { $text = ExtMobileFrontend::DOMParse( $out, $text, $context->isBetaGroupMember() ); @@ -206,6 +209,9 @@ } } + if ( !$title->isMainPage() && !$title->isSpecialPage() ) { + $text = MobileFrontendSkinHooks::interimTogglingSupport() . $text; + } return true; } diff --git a/includes/MobileFrontend.skin.hooks.php b/includes/MobileFrontend.skin.hooks.php index 2589819..0f73c69 100644 --- a/includes/MobileFrontend.skin.hooks.php +++ b/includes/MobileFrontend.skin.hooks.php @@ -2,6 +2,28 @@ class MobileFrontendSkinHooks { /** + * Make it possible to open sections while JavaScript is still loading. + * + * @return string The JavaScript code to add event handlers to the skin + */ + public static function interimTogglingSupport() { + $js = <<<JAVASCRIPT +function mfTempOpenSection( id ) { + var block = document.getElementById( "mf-section-" + id ); + block.className += " open-block"; + // The previous sibling to the content block is guaranteed to be the + // associated heading due to mobileformatter. We need to add the same + // class to flip the collapse arrow icon. + // <h[1-6]>heading</h[1-6]><div id="mf-section-[1-9]+"></div> + block.previousSibling.className += " open-block"; +} +JAVASCRIPT; + return Html::inlineScript( + ResourceLoader::filter( 'minify-js', $js ) + ); + } + + /** * Fallback for Grade C to load lazyload image placeholders. * * Note: This will add a single repaint for Grade C browsers as diff --git a/includes/api/ApiParseExtender.php b/includes/api/ApiParseExtender.php index 5850cd8..8ea59e4 100644 --- a/includes/api/ApiParseExtender.php +++ b/includes/api/ApiParseExtender.php @@ -88,6 +88,7 @@ $mf->setRemoveMedia( $params['noimages'] ); $mf->setIsMainPage( $params['mainpage'] && $mfSpecialCaseMainPage ); $mf->enableExpandableSections( !$params['mainpage'] ); + $mf->disableScripts(); // HACK: need a nice way to request a TOC- and edit link-free HTML in the first place // FIXME: Should this be .mw-editsection? $mf->remove( [ '.toc', 'mw-editsection', '.mw-headline-anchor' ] ); diff --git a/resources/mobile.toggle/toggle.less b/resources/mobile.toggle/toggle.less index 0c398d7..98d1cde 100644 --- a/resources/mobile.toggle/toggle.less +++ b/resources/mobile.toggle/toggle.less @@ -25,11 +25,6 @@ .collapsible-block { // bug 41401 - without this content doesn't always take up whole width width: 100%; - display: none; - - &.open-block { - display: block; - } } } diff --git a/resources/skins.minerva.content.styles/main.less b/resources/skins.minerva.content.styles/main.less index 5824802..27fa974 100644 --- a/resources/skins.minerva.content.styles/main.less +++ b/resources/skins.minerva.content.styles/main.less @@ -44,6 +44,16 @@ } } +.client-js { + .collapsible-block { + display: none; + + &.open-block { + display: block; + } + } +} + .nomobile { // No mobile should trump any other class. e.g. .content table { display: table; } display: none !important; diff --git a/resources/skins.minerva.toggling/init.js b/resources/skins.minerva.toggling/init.js index 834caeb..b0a9a75 100644 --- a/resources/skins.minerva.toggling/init.js +++ b/resources/skins.minerva.toggling/init.js @@ -14,7 +14,13 @@ */ function init( $container, prefix, page ) { // distinguish headings in content from other headings - $container.find( '> h1,> h2,> h3,> h4,> h5,> h6' ).addClass( 'section-heading' ); + $container.find( '> h1,> h2,> h3,> h4,> h5,> h6' ).addClass( 'section-heading' ) + .removeAttr( 'onclick' ); + // cleanup global as it is no longer needed. We check if it's undefined because + // there is no guarantee this won't be run on other skins e.g. Vector or cached HTML + if ( window.mfTempOpenSection !== undefined ) { + delete window.mfTempOpenSection; + } new Toggler( $container, prefix, page ); } diff --git a/tests/phpunit/MobileFormatterTest.php b/tests/phpunit/MobileFormatterTest.php index 2bd7aba..270153f 100644 --- a/tests/phpunit/MobileFormatterTest.php +++ b/tests/phpunit/MobileFormatterTest.php @@ -14,11 +14,14 @@ * * @param string $heading * @param string $innerHtml of the heading element + * @param integer [$sectionNumber] heading corresponds to * @return string */ - private function makeSectionHeading( $heading, $innerHtml ) { - return "<$heading class=\"section-heading\">" . self::SECTION_INDICATOR . - "$innerHtml</$heading>"; + private function makeSectionHeading( $heading, $innerHtml, $sectionNumber=1 ) { + return "<$heading class=\"section-heading\"" + . " onclick=\"javascript:mfTempOpenSection($sectionNumber)\">" + . self::SECTION_INDICATOR + . "$innerHtml</$heading>"; } /** @@ -31,7 +34,13 @@ */ private function makeSectionHtml( $sectionNumber, $contentHtml='', $isReferenceSection=false ) { $attrs = $isReferenceSection ? ' data-is-reference-section="1"' : ''; - return "<div class=\"mf-section-$sectionNumber\" id=\"mf-section-$sectionNumber\"" + $className = "mf-section-$sectionNumber"; + + if ( $sectionNumber > 0 ) { + $className = $className . ' collapsible-block'; + } + + return "<div class=\"$className\" id=\"mf-section-$sectionNumber\"" . "$attrs>$contentHtml</div>"; } @@ -126,7 +135,7 @@ $this->makeSectionHtml( 0, '<p>' . $originalImage . '</p>' ) . $this->makeSectionHeading( 'h2', 'heading 1' ) . $this->makeSectionHtml( 1, '<p>text</p>' ) - . $this->makeSectionHeading( 'h2', 'heading 2' ) + . $this->makeSectionHeading( 'h2', 'heading 2', 2 ) . $this->makeSectionHtml( 2, 'abc' ), $enableSections, false, false, true, @@ -140,7 +149,7 @@ . $this->makeSectionHtml( 1, '<p>text</p>' . $noscript . $placeholder ) - . $this->makeSectionHeading( 'h2', 'heading 2' ) + . $this->makeSectionHeading( 'h2', 'heading 2', 2 ) . $this->makeSectionHtml( 2, 'abc' ), $enableSections, false, false, true, @@ -154,7 +163,7 @@ . $this->makeSectionHtml( 1, '<p>text</p>' . $noscriptStyles . $placeholderStyles ) - . $this->makeSectionHeading( 'h2', 'heading 2' ) + . $this->makeSectionHeading( 'h2', 'heading 2', 2 ) . $this->makeSectionHtml( 2, 'abc' ), $enableSections, false, false, true, @@ -168,7 +177,7 @@ . $this->makeSectionHtml( 1, '<p>text</p>' . $noscript . $placeholder ) - . $this->makeSectionHeading( 'h2', 'heading 2' ) + . $this->makeSectionHeading( 'h2', 'heading 2', 2 ) . $this->makeSectionHtml( 2, $noscript . $placeholder ), $enableSections, false, false, true, @@ -695,7 +704,7 @@ $this->makeSectionHtml( 0, '' ) . $this->makeSectionHeading( 'h1', 'Foo' ) . $this->makeSectionHtml( 1, '<h2 class="in-block">Bar</h2>Baz' ) - . $this->makeSectionHeading( 'h1', 'Qux' ) + . $this->makeSectionHeading( 'h1', 'Qux', 2 ) . $this->makeSectionHtml( 2, 'Quux' ), ], ]; -- To view, visit https://gerrit.wikimedia.org/r/319067 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia342b3ddfbf0a419122aa0de282310396fa264e7 Gerrit-PatchSet: 10 Gerrit-Project: mediawiki/extensions/MobileFrontend Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org> Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Jhobs <jhob...@wikimedia.org> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits