From cb7ca112747c34f8841b6577cc4c5aeca1e0981a Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Thu, 21 May 2020 11:49:24 -0700 Subject: [PATCH] Reduce distribution of legacy classes The .menu class historically only needs to apply to dropdowns. the .vectorMenuCheckbox is inconsistent with the other classes on the menu so we should begin its deprecation. Bug: T253329 Change-Id: I00b4d2fd795195cd9c8add650a3b3cafdced5465 --- includes/VectorTemplate.php | 7 +++++++ includes/templates/Menu.mustache | 4 ++-- resources/skins.vector.styles.responsive.less | 4 +++- resources/skins.vector.styles/MenuDropdown.less | 8 ++++++-- stories/Menu.stories.data.js | 4 ++++ stories/MenuDropdown.stories.data.js | 2 ++ stories/MenuPortal.stories.data.js | 5 +++++ stories/MenuTabs.stories.data.js | 2 ++ stories/types.js | 1 + tests/phpunit/integration/VectorTemplateTest.php | 1 + 10 files changed, 33 insertions(+), 5 deletions(-) diff --git a/includes/VectorTemplate.php b/includes/VectorTemplate.php index a7dd37e8..314d5b80 100644 --- a/includes/VectorTemplate.php +++ b/includes/VectorTemplate.php @@ -399,6 +399,12 @@ class VectorTemplate extends BaseTemplate { self::MENU_TYPE_PORTAL => 'vector-menu vector-menu-portal portal', self::MENU_TYPE_DEFAULT => 'vector-menu', ]; + // A list of classes to apply the list element and override the default behavior. + $listClasses = [ + // `.menu` is on the portal for historic reasons. + // It should not be applied elsewhere per T253329. + self::MENU_TYPE_DROPDOWN => 'menu vector-menu-content-list', + ]; $isPortal = self::MENU_TYPE_PORTAL === $type; // For some menu items, there is no language key corresponding with its menu key. @@ -410,6 +416,7 @@ class VectorTemplate extends BaseTemplate { // If no message exists fallback to plain text (T252727) 'label' => $msgObj->exists() ? $msgObj->text() : $label, 'html-userlangattributes' => $this->get( 'userlangattributes', '' ), + 'list-classes' => $listClasses[$type] ?? 'vector-menu-content-list', 'html-items' => '', 'is-dropdown' => self::MENU_TYPE_DROPDOWN === $type, 'html-tooltip' => Linker::tooltip( 'p-' . $label ), diff --git a/includes/templates/Menu.mustache b/includes/templates/Menu.mustache index c007c378..985b778b 100644 --- a/includes/templates/Menu.mustache +++ b/includes/templates/Menu.mustache @@ -4,7 +4,7 @@
{{#is-dropdown}} - + {{/is-dropdown}}

{{label}} @@ -14,7 +14,7 @@
-
    {{{html-items}}}
+
    {{{html-items}}}
{{{html-after-portal}}}

diff --git a/resources/skins.vector.styles.responsive.less b/resources/skins.vector.styles.responsive.less index 25e46ed6..3bcd76f4 100644 --- a/resources/skins.vector.styles.responsive.less +++ b/resources/skins.vector.styles.responsive.less @@ -108,7 +108,9 @@ padding: 0; } - .vectorMenu .menu { + // FIXME: Drop .menu class after cache has cleared. + .vectorMenu .menu, + .vectorMenu .vector-menu-content-list { left: inherit; right: -1px; } diff --git a/resources/skins.vector.styles/MenuDropdown.less b/resources/skins.vector.styles/MenuDropdown.less index 54c6fd6d..15733872 100644 --- a/resources/skins.vector.styles/MenuDropdown.less +++ b/resources/skins.vector.styles/MenuDropdown.less @@ -67,14 +67,18 @@ z-index: @z-index-menu; } - &:hover .menu { + // FIXME: Drop .menu class after cache has cleared. + &:hover .menu, + &:hover .vector-menu-content-list { opacity: 1; visibility: visible; } // This is in a separate block, so that browsers supporting :hover but not :checked still apply the rule above // Support: IE8 - .vectorMenuCheckbox:checked ~ .menu { + // FIXME: drop .menu selector when cache has cleared + .vectorMenuCheckbox:checked ~ .menu, + .vector-menu-checkbox:checked ~ .vector-menu-content-list { opacity: 1; visibility: visible; } diff --git a/stories/Menu.stories.data.js b/stories/Menu.stories.data.js index 85e1461f..0712ca1f 100644 --- a/stories/Menu.stories.data.js +++ b/stories/Menu.stories.data.js @@ -7,6 +7,7 @@ import { htmluserlangattributes } from './utils'; const loggedOut = { id: 'p-personal', class: 'vector-menu', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-personal-label', label: 'Personal tools', 'html-userlangattributes': htmluserlangattributes, @@ -19,6 +20,7 @@ const loggedOut = { const loggedInWithEcho = { id: 'p-personal', class: 'vector-menu', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-personal-label', label: 'Personal tools', 'html-userlangattributes': htmluserlangattributes, @@ -33,6 +35,7 @@ const ULS_LANGUAGE_SELECTOR = '
  • Englis const defaultMenu = { id: 'p-generic', class: 'vector-menu', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-generic-label', label: 'Menu label', 'html-userlangattributes': htmluserlangattributes, @@ -47,6 +50,7 @@ const defaultMenu = { const loggedInWithULS = { id: 'p-personal', class: 'vector-menu', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-personal-label', label: 'Personal tools', 'html-userlangattributes': htmluserlangattributes, diff --git a/stories/MenuDropdown.stories.data.js b/stories/MenuDropdown.stories.data.js index 4397ca13..272f07ad 100644 --- a/stories/MenuDropdown.stories.data.js +++ b/stories/MenuDropdown.stories.data.js @@ -9,6 +9,7 @@ export { vectorMenuTemplate }; export const moreData = { 'is-dropdown': true, class: 'vector-menu-dropdown', + 'list-classes': 'vector-menu-content-list', label: 'More', id: 'p-cactions', 'label-id': 'p-cactions-label', @@ -33,6 +34,7 @@ export const moreData = { export const variantsData = { 'is-dropdown': true, class: 'vector-menu-dropdown', + 'list-classes': 'vector-menu-content-list', label: '新加坡简体', id: 'p-variants', 'label-id': 'p-variants-label', diff --git a/stories/MenuPortal.stories.data.js b/stories/MenuPortal.stories.data.js index fa8cd093..db45f504 100644 --- a/stories/MenuPortal.stories.data.js +++ b/stories/MenuPortal.stories.data.js @@ -30,6 +30,7 @@ export const PORTALS = { example: { id: 'p-example', class: 'vector-menu-portal portal', + 'list-classes': 'vector-menu-content-list', 'html-tooltip': 'Message tooltip-p-example acts as tooltip', label: 'Portal title', 'label-id': 'p-example-label', @@ -46,6 +47,7 @@ export const PORTALS = { navigation: { id: 'p-navigation', class: 'portal portal-first', + 'list-classes': 'vector-menu-content-list', 'html-tooltip': 'A message tooltip-p-navigation must exist for this to appear', label: 'Navigation', 'label-id': 'p-navigation-label', @@ -58,6 +60,7 @@ export const PORTALS = { toolbox: { id: 'p-tb', class: 'vector-menu-portal portal', + 'list-classes': 'vector-menu-content-list', 'html-tooltip': 'A message tooltip-p-tb must exist for this to appear', label: 'Tools', 'label-id': 'p-tb-label', @@ -70,6 +73,7 @@ export const PORTALS = { langlinks: { id: 'p-lang', class: 'vector-menu-portal portal', + 'list-classes': 'vector-menu-content-list', 'html-tooltip': 'A message tooltip-p-lang must exist for this to appear', label: 'In other languages', 'label-id': 'p-lang-label', @@ -88,6 +92,7 @@ ${placeholder( `

    Further hook output possible (lang)

    `, 60 )}` otherProjects: { id: 'p-wikibase-otherprojects', class: 'vector-menu-portal portal', + 'list-classes': 'vector-menu-content-list', 'html-tooltip': 'A message tooltip-p-wikibase-otherprojects must exist for this to appear', label: 'In other projects', 'label-id': 'p-wikibase-otherprojects-label', diff --git a/stories/MenuTabs.stories.data.js b/stories/MenuTabs.stories.data.js index 045455c1..b13d2e85 100644 --- a/stories/MenuTabs.stories.data.js +++ b/stories/MenuTabs.stories.data.js @@ -6,6 +6,7 @@ import { htmluserlangattributes } from './utils'; export const pageActionsData = { id: 'p-views', class: 'vector-menu-tabs vectorTabs', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-views-label', label: 'Views', 'html-userlangattributes': htmluserlangattributes, @@ -28,6 +29,7 @@ You can view its source [⌃⌥e]" accesskey="e">View source
    • export const namespaceTabsData = { id: 'p-namespaces', class: 'vector-menu-tabs vectorTabs', + 'list-classes': 'vector-menu-content-list', 'label-id': 'p-namespaces-label', label: 'Namespaces', 'html-userlangattributes': htmluserlangattributes, diff --git a/stories/types.js b/stories/types.js index 0c9ca11a..739851b1 100644 --- a/stories/types.js +++ b/stories/types.js @@ -30,6 +30,7 @@ * @prop {string} html-items * @prop {string} [html-tooltip] * @prop {string} [class] of menu + * @prop {string} list-classes of the unordered list element inside the menu * @prop {string} [html-userlangattributes] * @prop {boolean} [is-dropdown] * @prop {string} [html-hook-vector-after-toolbox] Deprecated and used by the toolbox portal menu. diff --git a/tests/phpunit/integration/VectorTemplateTest.php b/tests/phpunit/integration/VectorTemplateTest.php index 20e2819e..9ccbc442 100644 --- a/tests/phpunit/integration/VectorTemplateTest.php +++ b/tests/phpunit/integration/VectorTemplateTest.php @@ -117,6 +117,7 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase { 'label-id' => 'p-views-label', 'label' => 'Views', 'html-userlangattributes' => $langAttrs, + 'list-classes' => 'vector-menu-content-list', 'html-items' => '', 'is-dropdown' => false, 'html-tooltip' => '',