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

Reply via email to