Refactor: Simplify and standardize menu definitions

* Standardise the menu markup. This means all menus in Vector will now
be wrapped in a div and will have a heading.
* All menus now have the vector-menu class. Styles specific to personal tools
are moved to layout since these are concerned with placement.
* The ul class will always have menu class.
* emptyPortal class is generalised into vector-menu-empty for consistency
with other classes and moved from common.less into Menu.less
* Standardise hooks - BaseTemplateAfterPortlet can now be run on any
menu.

Changes to HTML:
* lang and dir attributes are moved from the h3 up to the div element
.vectorTabs, .portal(s) and #p-personal now has hidden span element inside h3
* for non portals ul.menu" is now wrapped in a div.vector-menu-content

This change does impact the following CSS selectors which will need to be updated:

I see no matches for these selectors in code search.

```
 #p-variants > ul
 #p-namespaces > ul
 #p-personal > ul
 #p-views > ul
 #p-cactions > ul

```
Using global-search.toolforge.org I see one match
for p-variants, 26 for p-namespaces, 30 for p-personal,
36 for p-views and 7 for p-cactions. I see this as acceptable
breakage provided a user notice is sent out which it has been
(T252447)

Bug: T249372
Change-Id: Id59234aa6b822a24848386bdc04d8d7ed37ca145
This commit is contained in:
jdlrobson 2020-04-16 11:40:39 -07:00
parent 4e661112e8
commit e048c2a729
11 changed files with 67 additions and 49 deletions

View file

@ -445,13 +445,11 @@ class VectorTemplate extends BaseTemplate {
array $options = [], array $options = [],
bool $setLabelToSelected = false bool $setLabelToSelected = false
) : array { ) : array {
$class = ( count( $urls ) == 0 ) ? 'emptyPortlet' : ''; $class = ( count( $urls ) == 0 ) ? 'vector-menu-empty emptyPortlet' : '';
// FIXME: All menus should carry vector-menu, but this can only be done when
// Menu.less CSS has been generalised to not include layout.
$extraClasses = [ $extraClasses = [
self::MENU_TYPE_DROPDOWN => 'vector-menu-dropdown vectorMenu', self::MENU_TYPE_DROPDOWN => 'vector-menu vector-menu-dropdown vectorMenu',
self::MENU_TYPE_TABS => 'vector-menu-tabs vectorTabs', self::MENU_TYPE_TABS => 'vector-menu vector-menu-tabs vectorTabs',
self::MENU_TYPE_PORTAL => 'vector-menu-portal portal', self::MENU_TYPE_PORTAL => 'vector-menu vector-menu-portal portal',
self::MENU_TYPE_DEFAULT => 'vector-menu', self::MENU_TYPE_DEFAULT => 'vector-menu',
]; ];
$isPortal = self::MENU_TYPE_PORTAL === $type; $isPortal = self::MENU_TYPE_PORTAL === $type;
@ -467,7 +465,6 @@ class VectorTemplate extends BaseTemplate {
'html-items' => '', 'html-items' => '',
'is-dropdown' => self::MENU_TYPE_DROPDOWN === $type, 'is-dropdown' => self::MENU_TYPE_DROPDOWN === $type,
'html-tooltip' => Linker::tooltip( 'p-' . $label ), 'html-tooltip' => Linker::tooltip( 'p-' . $label ),
'is-portal' => $isPortal,
]; ];
foreach ( $urls as $key => $item ) { foreach ( $urls as $key => $item ) {
@ -483,7 +480,6 @@ class VectorTemplate extends BaseTemplate {
} }
$props['html-after-portal'] = $isPortal ? $this->getAfterPortlet( $label ) : ''; $props['html-after-portal'] = $isPortal ? $this->getAfterPortlet( $label ) : '';
return $props; return $props;
} }

View file

@ -1,27 +1,22 @@
{{! {{!
See @typedef MenuDefinition See @typedef MenuDefinition
}} }}
<div id="{{id}}" role="navigation" class="{{class}}" aria-labelledby="{{label-id}}" {{{html-tooltip}}}> <div id="{{id}}" class="{{class}}" aria-labelledby="{{label-id}}" {{{html-tooltip}}}
{{{html-userlangattributes}}}>
{{#is-dropdown}} {{#is-dropdown}}
<input type="checkbox" class="vectorMenuCheckbox" aria-labelledby="{{label-id}}" /> <input type="checkbox" class="vectorMenuCheckbox" aria-labelledby="{{label-id}}" />
<h3 id="{{label-id}}" {{{html-userlangattributes}}}> {{/is-dropdown}}
<h3 id="{{label-id}}">
<span>{{label}}</span> <span>{{label}}</span>
</h3> </h3>
{{/is-dropdown}} {{! `body` class for backwards compatibility but let editors know not to use
{{^is-dropdown}} it via HTML comment below: }}
<h3 id="{{label-id}}">{{label}}</h3> <!-- Please do not use the .body class, it is deprecated. -->
{{/is-dropdown}} <div class="body vector-menu-content">
{{#is-portal}} <!-- Please do not use the .menu class, it is deprecated. -->
<div class="body"> <ul class="menu vector-menu-content-list">{{{html-items}}}</ul>
<ul>{{{html-items}}}</ul>
{{{html-after-portal}}} {{{html-after-portal}}}
</div> </div>
{{/is-portal}}
{{^is-portal}}
<ul class="menu" {{{html-userlangattributes}}}>
{{{html-items}}}
</ul>
{{/is-portal}}
</div> </div>
{{! Note: html-hook-vector-after-toolbox is deprecated. }} {{! Note: html-hook-vector-after-toolbox is deprecated. }}
{{{html-hook-vector-after-toolbox}}} {{{html-hook-vector-after-toolbox}}}

View file

@ -28,8 +28,8 @@ function init() {
var expandedWidth; var expandedWidth;
// If the dropdown was hidden, show it // If the dropdown was hidden, show it
// eslint-disable-next-line no-jquery/no-class-state // eslint-disable-next-line no-jquery/no-class-state
if ( $cactions.hasClass( 'emptyPortlet' ) ) { if ( $cactions.hasClass( 'vector-menu-empty' ) ) {
$cactions.removeClass( 'emptyPortlet' ); $cactions.removeClass( 'vector-menu-empty' );
// Now that it is visible, force-render it virtually // Now that it is visible, force-render it virtually
// to get its expanded width, then shrink it 1px before we // to get its expanded width, then shrink it 1px before we
// yield from JS (which means the expansion won't be visible). // yield from JS (which means the expansion won't be visible).
@ -47,7 +47,7 @@ function init() {
// eslint-disable-next-line no-jquery/no-animate // eslint-disable-next-line no-jquery/no-animate
$cactions.find( 'h3' ).animate( { width: '1px' }, 'normal', function () { $cactions.find( 'h3' ).animate( { width: '1px' }, 'normal', function () {
$( this ).attr( 'style', '' ) $( this ).attr( 'style', '' )
.parent().addClass( 'emptyPortlet' ); .parent().addClass( 'vector-menu-empty' );
} ); } );
} }
} ) } )
@ -86,7 +86,7 @@ function init() {
// Always collapse if the "More" button is already shown. // Always collapse if the "More" button is already shown.
// eslint-disable-next-line no-jquery/no-class-state // eslint-disable-next-line no-jquery/no-class-state
if ( !$cactions.hasClass( 'emptyPortlet' ) ) { if ( !$cactions.hasClass( 'vector-menu-empty' ) ) {
return true; return true;
} }

View file

@ -2,7 +2,9 @@
@import 'mediawiki.mixins.less'; @import 'mediawiki.mixins.less';
/* Hide empty portlets */ /* Hide empty portlets */
.emptyPortlet { // FIXME: Remove when cache cleared.
.emptyPortlet,
.vector-menu-empty {
display: none; display: none;
} }
@ -10,11 +12,8 @@
.vector-menu, .vector-menu,
/* FIXME: Remove p-personal selector when cache has cleared. */ /* FIXME: Remove p-personal selector when cache has cleared. */
#p-personal { #p-personal {
position: absolute; // Hidden by default, but displayed by certain menus
top: @top-personal-tools; // e.g. MenuPortal
right: 0.75em;
z-index: @z-index-personal;
h3 { h3 {
display: none; display: none;
} }
@ -22,16 +21,13 @@
ul { ul {
list-style: none none; list-style: none none;
margin: 0; margin: 0;
padding-left: 10em; /* Keep from overlapping logo */
} }
li { li {
float: left;
margin-left: 0.75em; margin-left: 0.75em;
// `padding-top` instead of `margin-top` necessary for // `padding-top` instead of `margin-top` necessary for
// anonymous user icon position below // anonymous user icon position below
padding-top: 0.5em; padding-top: 0.5em;
font-size: @font-size-nav-personal;
line-height: @line-height-nav-personal; line-height: @line-height-nav-personal;
white-space: nowrap; white-space: nowrap;
} }

View file

@ -9,6 +9,7 @@
direction: ltr; direction: ltr;
h3 { h3 {
display: block;
background-image: url( images/portal-separator.png ); // Support: IE 8 & 9, Fx 3.6-15, Safari 5.1-6, Chrome 10-25 background-image: url( images/portal-separator.png ); // Support: IE 8 & 9, Fx 3.6-15, Safari 5.1-6, Chrome 10-25
background-image: linear-gradient( to right, transparent 0, #c8ccd1 35%, #c8ccd1 70%, transparent 100% ); // Standard (Firefox 16+, IE 10+, Safari 6.1+, Chrome 26+) background-image: linear-gradient( to right, transparent 0, #c8ccd1 35%, #c8ccd1 70%, transparent 100% ); // Standard (Firefox 16+, IE 10+, Safari 6.1+, Chrome 26+)
background-position: center bottom; background-position: center bottom;

View file

@ -48,3 +48,7 @@ body {
font-size: @font-size-heading-1; font-size: @font-size-heading-1;
} }
} }
#p-personal li {
font-size: @font-size-nav-personal;
}

View file

@ -131,6 +131,24 @@ body {
margin-top: @height-header; margin-top: @height-header;
} }
#p-personal {
position: absolute;
top: @top-personal-tools;
right: 0.75em;
z-index: @z-index-personal;
ul {
// Keep from spilling out into the first column
// For completeness this should also include the width of the logo.
// As a result it is possible for the personal tools to overlap the logo.
padding-left: @width-grid-column-one;
}
li {
float: left;
}
}
#mw-panel { #mw-panel {
position: absolute; position: absolute;
top: @height-header; top: @height-header;

View file

@ -25,6 +25,21 @@ body {
padding: @padding-content; padding: @padding-content;
} }
#p-personal {
position: absolute;
top: @top-personal-tools;
right: 0.75em;
z-index: @z-index-personal;
ul {
padding-left: 10em; /* Keep from overlapping logo */
}
li {
float: left;
}
}
.mw-body, .mw-body,
#mw-data-after-content { #mw-data-after-content {
margin-left: 10em; margin-left: 10em;

View file

@ -34,7 +34,6 @@ export const PORTALS = {
label: 'Portal title', label: 'Portal title',
'label-id': 'p-example-label', 'label-id': 'p-example-label',
'html-userlangattributes': htmluserlangattributes, 'html-userlangattributes': htmluserlangattributes,
'is-portal': true,
'html-items': ` 'html-items': `
<li><a href='#'>A list of links</a></li> <li><a href='#'>A list of links</a></li>
<li><a href='#'>with ids</a></li> <li><a href='#'>with ids</a></li>
@ -51,7 +50,6 @@ export const PORTALS = {
label: 'Navigation', label: 'Navigation',
'label-id': 'p-navigation-label', 'label-id': 'p-navigation-label',
'html-userlangattributes': htmluserlangattributes, 'html-userlangattributes': htmluserlangattributes,
'is-portal': true,
'html-items': ` 'html-items': `
<li id="n-mainpage-description"><a href="/wiki/Main_Page" title="Visit the main page [⌃⌥z]" accesskey="z">Main page</a></li><li id="n-contents"><a href="/wiki/Wikipedia:Contents" title="Guides to browsing Wikipedia">Contents</a></li><li id="n-featuredcontent"><a href="/wiki/Wikipedia:Featured_content" title="Featured content the best of Wikipedia">Featured content</a></li><li id="n-currentevents"><a href="/wiki/Portal:Current_events" title="Find background information on current events">Current events</a></li><li id="n-randompage"><a href="/wiki/Special:Random" title="Load a random page [x]" accesskey="x">Random page</a></li><li id="n-sitesupport"><a href="https://donate.wikimedia.org/wiki/Special:FundraiserRedirector?utm_source=donate&amp;utm_medium=sidebar&amp;utm_campaign=C13_en.wikipedia.org&amp;uselang=en" title="Support us">Donate</a></li><li id="n-shoplink"><a href="//shop.wikimedia.org" title="Visit the Wikipedia store">Wikipedia store</a></li> <li id="n-mainpage-description"><a href="/wiki/Main_Page" title="Visit the main page [⌃⌥z]" accesskey="z">Main page</a></li><li id="n-contents"><a href="/wiki/Wikipedia:Contents" title="Guides to browsing Wikipedia">Contents</a></li><li id="n-featuredcontent"><a href="/wiki/Wikipedia:Featured_content" title="Featured content the best of Wikipedia">Featured content</a></li><li id="n-currentevents"><a href="/wiki/Portal:Current_events" title="Find background information on current events">Current events</a></li><li id="n-randompage"><a href="/wiki/Special:Random" title="Load a random page [x]" accesskey="x">Random page</a></li><li id="n-sitesupport"><a href="https://donate.wikimedia.org/wiki/Special:FundraiserRedirector?utm_source=donate&amp;utm_medium=sidebar&amp;utm_campaign=C13_en.wikipedia.org&amp;uselang=en" title="Support us">Donate</a></li><li id="n-shoplink"><a href="//shop.wikimedia.org" title="Visit the Wikipedia store">Wikipedia store</a></li>
`, `,
@ -64,7 +62,6 @@ export const PORTALS = {
label: 'Tools', label: 'Tools',
'label-id': 'p-tb-label', 'label-id': 'p-tb-label',
'html-userlangattributes': htmluserlangattributes, 'html-userlangattributes': htmluserlangattributes,
'is-portal': true,
'html-items': ` 'html-items': `
<li id="t-whatlinkshere"><a href="/wiki/Special:WhatLinksHere/Spain" title="A list of all wiki pages that link here [⌃⌥j]" accesskey="j">What links here</a></li><li id="t-recentchangeslinked"><a href="/wiki/Special:RecentChangesLinked/Spain" rel="nofollow" title="Recent changes in pages linked from this page [k]" accesskey="k">Related changes</a></li><li id="t-upload"><a href="/wiki/Wikipedia:File_Upload_Wizard" title="Upload files [u]" accesskey="u">Upload file</a></li><li id="t-specialpages"><a href="/wiki/Special:SpecialPages" title="A list of all special pages [q]" accesskey="q">Special pages</a></li><li id="t-permalink"><a href="/w/index.php?title=Spain&amp;oldid=935087243" title="Permanent link to this revision of the page">Permanent link</a></li><li id="t-info"><a href="/w/index.php?title=Spain&amp;action=info" title="More information about this page">Page information</a></li><li id="t-wikibase"><a href="https://www.wikidata.org/wiki/Special:EntityPage/Q29" title="Link to connected data repository item [g]" accesskey="g">Wikidata item</a></li><li id="t-cite"><a href="/w/index.php?title=Special:CiteThisPage&amp;page=Spain&amp;id=935087243" title="Information on how to cite this page">Cite this page</a></li> <li id="t-whatlinkshere"><a href="/wiki/Special:WhatLinksHere/Spain" title="A list of all wiki pages that link here [⌃⌥j]" accesskey="j">What links here</a></li><li id="t-recentchangeslinked"><a href="/wiki/Special:RecentChangesLinked/Spain" rel="nofollow" title="Recent changes in pages linked from this page [k]" accesskey="k">Related changes</a></li><li id="t-upload"><a href="/wiki/Wikipedia:File_Upload_Wizard" title="Upload files [u]" accesskey="u">Upload file</a></li><li id="t-specialpages"><a href="/wiki/Special:SpecialPages" title="A list of all special pages [q]" accesskey="q">Special pages</a></li><li id="t-permalink"><a href="/w/index.php?title=Spain&amp;oldid=935087243" title="Permanent link to this revision of the page">Permanent link</a></li><li id="t-info"><a href="/w/index.php?title=Spain&amp;action=info" title="More information about this page">Page information</a></li><li id="t-wikibase"><a href="https://www.wikidata.org/wiki/Special:EntityPage/Q29" title="Link to connected data repository item [g]" accesskey="g">Wikidata item</a></li><li id="t-cite"><a href="/w/index.php?title=Special:CiteThisPage&amp;page=Spain&amp;id=935087243" title="Information on how to cite this page">Cite this page</a></li>
`, `,
@ -77,7 +74,6 @@ export const PORTALS = {
label: 'In other languages', label: 'In other languages',
'label-id': 'p-lang-label', 'label-id': 'p-lang-label',
'html-userlangattributes': htmluserlangattributes, 'html-userlangattributes': htmluserlangattributes,
'is-portal': true,
'html-items': ` 'html-items': `
<li class="interlanguage-link interwiki-ace"> <li class="interlanguage-link interwiki-ace">
<a href="https://ace.wikipedia.org/wiki/Seupanyo" <a href="https://ace.wikipedia.org/wiki/Seupanyo"
@ -97,7 +93,6 @@ ${placeholder( `<p>Further hook output possible (lang)</p>`, 60 )}`
'label-id': 'p-wikibase-otherprojects-label', 'label-id': 'p-wikibase-otherprojects-label',
'html-userlangattributes': htmluserlangattributes, 'html-userlangattributes': htmluserlangattributes,
'is-portal': true,
'html-items': ` 'html-items': `
<li class="wb-otherproject-link wb-otherproject-commons"><a href="https://commons.wikimedia.org/wiki/Category:Spain" hreflang="en">Wikimedia Commons</a></li><li class="wb-otherproject-link wb-otherproject-wikinews"><a href="https://en.wikinews.org/wiki/Category:Spain" hreflang="en">Wikinews</a></li><li class="wb-otherproject-link wb-otherproject-wikiquote"><a href="https://en.wikiquote.org/wiki/Spain" hreflang="en">Wikiquote</a></li><li class="wb-otherproject-link wb-otherproject-wikivoyage"><a href="https://en.wikivoyage.org/wiki/Spain" hreflang="en">Wikivoyage</a></li>`, <li class="wb-otherproject-link wb-otherproject-commons"><a href="https://commons.wikimedia.org/wiki/Category:Spain" hreflang="en">Wikimedia Commons</a></li><li class="wb-otherproject-link wb-otherproject-wikinews"><a href="https://en.wikinews.org/wiki/Category:Spain" hreflang="en">Wikinews</a></li><li class="wb-otherproject-link wb-otherproject-wikiquote"><a href="https://en.wikiquote.org/wiki/Spain" hreflang="en">Wikiquote</a></li><li class="wb-otherproject-link wb-otherproject-wikivoyage"><a href="https://en.wikivoyage.org/wiki/Spain" hreflang="en">Wikivoyage</a></li>`,
'html-after-portal': '' 'html-after-portal': ''

View file

@ -32,7 +32,6 @@
* @prop {string} [class] of menu * @prop {string} [class] of menu
* @prop {string} [html-userlangattributes] * @prop {string} [html-userlangattributes]
* @prop {boolean} [is-dropdown] * @prop {boolean} [is-dropdown]
* @prop {boolean} [is-portal]
* @prop {string} [html-hook-vector-after-toolbox] Deprecated and used by the toolbox portal menu. * @prop {string} [html-hook-vector-after-toolbox] Deprecated and used by the toolbox portal menu.
* @prop {string} [html-after-portal] Additional HTML specific to portal menus. * @prop {string} [html-after-portal] Additional HTML specific to portal menus.
*/ */

View file

@ -154,28 +154,27 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase {
$this->assertSame( $views, [ $this->assertSame( $views, [
'id' => 'p-views', 'id' => 'p-views',
'class' => 'emptyPortlet vector-menu-tabs vectorTabs', 'class' => 'vector-menu-empty emptyPortlet vector-menu vector-menu-tabs vectorTabs',
'label-id' => 'p-views-label', 'label-id' => 'p-views-label',
'label' => 'Views', 'label' => 'Views',
'html-userlangattributes' => $langAttrs, 'html-userlangattributes' => $langAttrs,
'html-items' => '', 'html-items' => '',
'class' => 'emptyPortlet vector-menu-tabs vectorTabs', 'class' => 'vector-menu-empty emptyPortlet vector-menu vector-menu-tabs vectorTabs',
'is-dropdown' => false, 'is-dropdown' => false,
'html-tooltip' => '', 'html-tooltip' => '',
'is-portal' => false,
'html-after-portal' => '' 'html-after-portal' => ''
] ); ] );
$variants = $props['data-variants']; $variants = $props['data-variants'];
$actions = $props['data-page-actions-more']; $actions = $props['data-page-actions-more'];
$this->assertSame( $namespaces['class'], $this->assertSame( $namespaces['class'],
'emptyPortlet vector-menu-tabs vectorTabs' ); 'vector-menu-empty emptyPortlet vector-menu vector-menu-tabs vectorTabs' );
$this->assertSame( $variants['class'], $this->assertSame( $variants['class'],
'emptyPortlet vector-menu-dropdown vectorMenu' ); 'vector-menu-empty emptyPortlet vector-menu vector-menu-dropdown vectorMenu' );
$this->assertSame( $actions['class'], $this->assertSame( $actions['class'],
'emptyPortlet vector-menu-dropdown vectorMenu' ); 'vector-menu-empty emptyPortlet vector-menu vector-menu-dropdown vectorMenu' );
$this->assertSame( $props['data-personal-menu']['class'], $this->assertSame( $props['data-personal-menu']['class'],
'emptyPortlet vector-menu' ); 'vector-menu-empty emptyPortlet vector-menu' );
} }
} }