sortable tables/mw-collapsible no longer causes page jump

Relatively straightforwardly - we move padding-right definition into
top styles to avoid a reflow

This turned out to be a little tricker than first imagined.
The code in jquery.makeCollapsible.js finds elements marked up
as collapsible and then 1) collapses them 2) inserts a toggle button.

To avoid a reflow on the page, we have to make sure the collapsed content
is collapsed prior to the code running and reserve space for the toggle
button.

The toggle button is tricky as it is subject to i18n. However we can
access this label via a LESS variable and use a pseudo element (which
is supported by the majority of modern browsers) to preserve the space
we need. When the JavaScript has loaded there is no need for that CSS
any more.

Note on slow connections, collapsed content will not be accessible until
the JavaScript has fully loaded, however this is arguably less of a problem
then the existing reflows.

Note, there are many many many usages of mw-collapsible, because of this
this does not claim to address all reflows in all usages - but it does provide
support for the more widespread usages.

Follow up patches can be added for specific situations - for example table
of contents toggle.

Bug: T42792
Bug: T42812
Depends-On:  I3388c3c4f91cdbab11e89cdc95973b688d3f1ce7
Change-Id: If9c8f0974e3a4b08e4a66d37f7f5adf67d73054e

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 9c4ac50..2247cc2 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -179,7 +179,9 @@ abstract class Skin extends ContextSource {
                        // Styles key sets render blocking styles
                        // Unlike other keys in this definition it is an associative array
                        // where each key is the group name and points to a list of modules
-                       'styles' => [],
+                       'styles' => [
+                               'content' => [],
+                       ],
                        // modules not specific to any specific skin or page
                        'core' => [
                                // Enforce various default modules for all pages and all skins
@@ -208,11 +210,13 @@ abstract class Skin extends ContextSource {
                // Preload jquery.tablesorter for mediawiki.page.ready
                if ( strpos( $out->getHTML(), 'sortable' ) !== false ) {
                        $modules['content'][] = 'jquery.tablesorter';
+                       $modules['styles']['content'][] = 'jquery.tablesorter.styles';
                }
 
                // Preload jquery.makeCollapsible for mediawiki.page.ready
                if ( strpos( $out->getHTML(), 'mw-collapsible' ) !== false ) {
                        $modules['content'][] = 'jquery.makeCollapsible';
+                       $modules['styles']['content'][] = 'jquery.makeCollapsible.styles';
                }
 
                if ( $out->isTOCEnabled() ) {

diff --git a/resources/Resources.php b/resources/Resources.php
index d3e1b65..12a7bf4 100644 (file)
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -113,6 +113,20 @@ return [
                ],
        ],
 
+       'jquery.tablesorter.styles' => [
+               'targets' => [ 'desktop', 'mobile' ],
+               'styles' => [
+                       'resources/src/jquery/jquery.tablesorter.styles.less',
+               ],
+       ],
+       'jquery.makeCollapsible.styles' => [
+               'targets' => [ 'desktop', 'mobile' ],
+               'class' => ResourceLoaderLessVarFileModule::class,
+               'styles' => [
+                       'resources/src/jquery/jquery.makeCollapsible.styles.less',
+               ],
+       ],
+
        'mediawiki.skinning.content.parsoid' => [
                // Style Parsoid HTML+RDFa output consistent with wikitext from PHP parser
                // with the interface.css styles; skinStyles should be used if your
@@ -277,6 +291,7 @@ return [
                'scripts' => 'resources/src/jquery/jquery.localize.js',
        ],
        'jquery.makeCollapsible' => [
+               'dependencies' => [ 'jquery.makeCollapsible.styles' ],
                'scripts' => 'resources/src/jquery/jquery.makeCollapsible.js',
                'styles' => 'resources/src/jquery/jquery.makeCollapsible.css',
                'messages' => [ 'collapsible-expand', 'collapsible-collapse' ],
@@ -317,6 +332,7 @@ return [
                'styles' => 'resources/src/jquery/jquery.tablesorter.less',
                'messages' => [ 'sort-descending', 'sort-ascending' ],
                'dependencies' => [
+                       'jquery.tablesorter.styles',
                        'mediawiki.RegExp',
                        'mediawiki.language.months',
                ],

diff --git a/resources/src/jquery/jquery.makeCollapsible.css b/resources/src/jquery/jquery.makeCollapsible.css
index 6364c70..fc52d51 100644 (file)
--- a/resources/src/jquery/jquery.makeCollapsible.css
+++ b/resources/src/jquery/jquery.makeCollapsible.css
@@ -1,3 +1,9 @@
+/*
+ * Please do not add any CSS rules here that impact the positioning of the element
+ *  e.g. padding, margin, position or float.
+ * These instead should live in jquery.makeCollapsible.styles
+*/
+
 /* See also jquery.makeCollapsible.js */
 .mw-collapsible-toggle {
        float: right;
@@ -12,17 +18,6 @@
 .mw-collapsible-toggle-default:after {
        content: ']';
 }
-/* Align the toggle based on the direction of the content language */
-/* @noflip */
-.mw-content-ltr .mw-collapsible-toggle,
-.mw-content-rtl .mw-content-ltr .mw-collapsible-toggle {
-       float: right;
-}
-/* @noflip */
-.mw-content-rtl .mw-collapsible-toggle,
-.mw-content-ltr .mw-content-rtl .mw-collapsible-toggle {
-       float: left;
-}
 
 .mw-customtoggle,
 .mw-collapsible-toggle {
@@ -37,17 +32,3 @@ caption .mw-collapsible-toggle,
 .mw-content-ltr .mw-content-rtl caption .mw-collapsible-toggle {
        float: none;
 }
-
-/* list-items go as wide as their parent element, don't float them inside list items */
-li .mw-collapsible-toggle,
-.mw-content-ltr li .mw-collapsible-toggle,
-.mw-content-rtl li .mw-collapsible-toggle,
-.mw-content-rtl .mw-content-ltr li .mw-collapsible-toggle,
-.mw-content-ltr .mw-content-rtl li .mw-collapsible-toggle {
-       float: none;
-}
-
-/* the added list item should have no list-style */
-.mw-collapsible-toggle-li {
-       list-style: none;
-}

diff --git a/resources/src/jquery/jquery.makeCollapsible.js b/resources/src/jquery/jquery.makeCollapsible.js
index 1f40e0a..503e3a6 100644 (file)
--- a/resources/src/jquery/jquery.makeCollapsible.js
+++ b/resources/src/jquery/jquery.makeCollapsible.js
@@ -1,5 +1,8 @@
 /**
  * jQuery makeCollapsible
+ * Note: To avoid performance issues such as reflows, several styles are
+ * shipped in mediawiki.makeCollapsible.styles to reserve space for the toggle control. Please
+ * familiarise yourself with that CSS before making any changes to this code.
  *
  * Dual licensed:
  * - CC BY 3.0 <http://creativecommons.org/licenses/by/3.0>
@@ -242,7 +245,11 @@
                        if ( $collapsible.data( 'mw-made-collapsible' ) ) {
                                return;
                        } else {
-                               $collapsible.data( 'mw-made-collapsible', true );
+                               // Let CSS know that it no longer needs to worry about flash of unstyled content.
+                               // This will allow mediawiki.makeCollapsible.styles to disable temporary pseudo elements, that
+                               // are needed to avoid a flash of unstyled content.
+                               $collapsible.addClass( 'mw-made-collapsible' )
+                                       .data( 'mw-made-collapsible', true );
                        }
 
                        // Use custom text or default?

diff --git a/resources/src/jquery/jquery.makeCollapsible.styles.less b/resources/src/jquery/jquery.makeCollapsible.styles.less
new file mode 100644 (file)
index 0000000..f19c3c2
--- /dev/null
+++ b/resources/src/jquery/jquery.makeCollapsible.styles.less
@@ -0,0 +1,125 @@
+/**
+ * These rules prevent re-flows relating to collapsible on-wiki elements (T42812).
+ * This is done by temporarily creating a pseudo element in the place that JavaScript will insert
+ * a toggle control. The same CSS rules that control the positioning of the toggle control will apply
+ * to the pseudo element. When the JavaScript has executed
+ * (See corresponding non-render blocking CSS in jquery.makeCollapsible)
+ * all pseudo elements will be removed.
+ *
+ * Currently we support all the examples on [[mw:Manual:Collapsible_elements/Demo/Simple]]
+ * All examples on [[mw:Manual:Collapsible_elements/Demo/Advanced]] are supported with the following
+ * exceptions
+ * - Custom collapsible 4 (table-row)
+ * -- CSS selector would be too complicated
+ * - Collapsible div nested in collapsed div
+ * -- Given it's not easy to identify the collapsed content via CSS, text will be shown until
+ *    JavaScript has loaded
+ * - "Combination example"
+ * -- At a later time, we may want to support the use of of `attr`, but given the code
+ *    complexity we will not for the time being (see https://davidwalsh.name/css-content-attr)
+ */
+
+// This selector is used frequently in the code to indicate that the JavaScript has successfully completed
+// it's execution and pseudo elements can be disabled. For readability and maintainability it is separated
+// as a LESS variable.
+@exclude: ~'.mw-made-collapsible';
+
+.client-js {
+
+       ol.mw-collapsible:before,
+       ul.mw-collapsible:before,
+       .mw-collapsible-toggle-li {
+               /*
+               Rather than inherit any margins from the the general li selector - make sure this is explicit
+               to avoid reflows
+               */
+               display: list-item;
+               list-style: none;
+               margin-bottom: 0.1em;
+       }
+
+       // Reset when mw-collapsible-toggle-li is rendered
+       ol.mw-made-collapsible:before,
+       ul.mw-made-collapsible:before {
+               display: none;
+       }
+
+       ol.mw-collapsible:not( @{exclude} ):before,
+       ul.mw-collapsible:not( @{exclude} ):before,
+       // Where the tbody or thead is the first child of the collapsible table
+       table.mw-collapsible:not( @{exclude} ) :first-child tr:first-child th:last-child:before,
+       table.mw-collapsible:not( @{exclude} ) > caption:first-child:after {
+               content: '[@{msg-collapsible-collapse}]';
+       }
+
+       td.mw-collapsed:not( @{exclude} ):before,
+       table.mw-collapsed:not( @{exclude} ) :first-child tr:first-child th:last-child:before,
+       table.mw-collapsed:not( @{exclude} ) > caption:first-child:after,
+       div.mw-collapsed:not( @{exclude} ):before {
+               content: '[@{msg-collapsible-expand}]';
+       }
+
+       // Any element with id beginning `mw-customcollapsible` will have special treatment
+       .mw-collapsible[ id^='mw-customcollapsible' ] th:before,
+       .mw-collapsible[ id^='mw-customcollapsible' ]:before {
+               content: none !important; // stylelint-disable-line declaration-no-important
+       }
+
+       // Special case for table where first child is caption element
+       table.mw-collapsible:not( @{exclude} ) > caption:first-child:after {
+               float: none;
+               display: block;
+       }
+
+       // Use the exclude selector to ensure animations do not break
+       .mw-collapsed:not( @{exclude} ) {
+               // Avoid FOUC/reflows on collapsed elements by making sure they are opened by default (T42812)
+               > p,
+               > table,
+               // Manual:Collapsible_elements/Demo/Simple#Collapsed_by_default
+               > thead + tbody,
+               tr:not( :first-child ),
+               .mw-collapsible-content {
+                       display: none;
+               }
+       }
+}
+
+/* Align the toggle based on the direction of the content language */
+/* @noflip */
+.mw-content-ltr,
+.mw-content-rtl .mw-content-ltr {
+       .mw-collapsible:not( @{exclude} ) th:before,
+       .mw-collapsible:not( @{exclude} ):before,
+       .mw-collapsible-toggle {
+               float: right;
+       }
+}
+
+/* @noflip */
+.mw-content-rtl,
+.mw-content-ltr .mw-content-rtl {
+       .mw-collapsible:not( @{exclude} ) th:before,
+       .mw-collapsible:not( @{exclude} ):before,
+       .mw-collapsible-toggle {
+               float: left;
+       }
+}
+
+/* list-items go as wide as their parent element, don't float them inside list items */
+li,
+.mw-content-ltr li,
+.mw-content-rtl li,
+.mw-content-ltr .mw-content-rtl li,
+.mw-content-rtl .mw-content-ltr li {
+       .mw-collapsible-toggle {
+               float: none;
+       }
+}
+
+// special treatment for list items to match above
+// !important necessary to override overly-specific float left and right above.
+ol.mw-collapsible:not( @{exclude} ):before,
+ul.mw-collapsible:not( @{exclude} ):before {
+       float: none !important; // stylelint-disable-line declaration-no-important
+}

diff --git a/resources/src/jquery/jquery.tablesorter.less b/resources/src/jquery/jquery.tablesorter.less
index ce24b0d..3bea471 100644 (file)
--- a/resources/src/jquery/jquery.tablesorter.less
+++ b/resources/src/jquery/jquery.tablesorter.less
@@ -8,7 +8,10 @@ table.jquery-tablesorter {
                cursor: pointer;
                background-repeat: no-repeat;
                background-position: center right;
-               padding-right: 21px;
+               // Note: To avoid reflows, a padding is set in
+               // the jquery.tableSorter.styles module as a render blocking style.
+               // Please do not add any CSS rules here that impact the positioning of the element
+               // e.g. padding, margin, position or float.
        }
 
        th.headerSortUp {

diff --git a/resources/src/jquery/jquery.tablesorter.styles.less b/resources/src/jquery/jquery.tablesorter.styles.less
new file mode 100644 (file)
index 0000000..bd6b5dd
--- /dev/null
+++ b/resources/src/jquery/jquery.tablesorter.styles.less
@@ -0,0 +1,6 @@
+.client-js {
+       // Reserve space for table sortable controls
+       table.sortable th {
+               padding-right: 21px;
+       }
+}