From 872519ab940b608c9b54a4fd3e2b8c75f249eaba Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Wed, 25 Mar 2020 13:57:46 -0700 Subject: [PATCH] Drop the Navigation component It is not the most useful of components and adds an additional layer of complexity similar to multiple inheritance chains that we find in Object oriented programming. I suggest we use index.mustache going forward for laying out the different components and use components/template partials for reusable components. Change-Id: I6fd5fe1c3d3826d737ccd8ed5a38890305664876 --- includes/VectorTemplate.php | 34 +++++----- includes/templates/Navigation.mustache | 41 ------------ includes/templates/index.mustache | 43 +++++++++++-- resources/skins.vector.styles/Navigation.less | 32 ---------- resources/skins.vector.styles/common.less | 31 ++++++++++ resources/skins.vector.styles/index.less | 1 - resources/skins.vector.styles/legacy.less | 1 - stories/Navigation.stories.data.js | 49 --------------- stories/Navigation.stories.js | 20 ------ stories/skin.stories.js | 62 +++++++++++++++---- 10 files changed, 136 insertions(+), 178 deletions(-) delete mode 100644 includes/templates/Navigation.mustache delete mode 100644 resources/skins.vector.styles/Navigation.less delete mode 100644 stories/Navigation.stories.data.js delete mode 100644 stories/Navigation.stories.js diff --git a/includes/VectorTemplate.php b/includes/VectorTemplate.php index ca38605..722b906 100644 --- a/includes/VectorTemplate.php +++ b/includes/VectorTemplate.php @@ -175,29 +175,27 @@ class VectorTemplate extends BaseTemplate { 'html-hook-vector-before-footer' => $htmlHookVectorBeforeFooter, 'array-footer-rows' => $this->getTemplateFooterRows(), ], - 'data-navigation' => [ - 'html-navigation-heading' => $this->getMsg( 'navigation-heading' ), - 'data-personal-menu' => $this->buildPersonalProps(), - 'data-namespace-tabs' => $this->buildNamespacesProps(), - 'data-variants' => $this->buildVariantsProps(), - 'data-page-actions' => $this->buildViewsProps(), - 'data-page-actions-more' => $this->buildActionsProps(), - 'data-search-box' => $this->buildSearchProps(), - 'data-sidebar' => [ - 'html-logo-attributes' => Xml::expandAttributes( - Linker::tooltipAndAccesskeyAttribs( 'p-logo' ) + [ - 'class' => 'mw-wiki-logo', - 'href' => Skin::makeMainPageUrl(), - ] - ) - ] + $this->buildSidebarProps( $this->get( 'sidebar', [] ) ), - ], + 'html-navigation-heading' => $this->getMsg( 'navigation-heading' ), + 'data-personal-menu' => $this->buildPersonalProps(), + 'data-namespace-tabs' => $this->buildNamespacesProps(), + 'data-variants' => $this->buildVariantsProps(), + 'data-page-actions' => $this->buildViewsProps(), + 'data-page-actions-more' => $this->buildActionsProps(), + 'data-search-box' => $this->buildSearchProps(), + 'data-sidebar' => [ + 'html-logo-attributes' => Xml::expandAttributes( + Linker::tooltipAndAccesskeyAttribs( 'p-logo' ) + [ + 'class' => 'mw-wiki-logo', + 'href' => Skin::makeMainPageUrl(), + ] + ) + ] + $this->buildSidebarProps( $this->get( 'sidebar', [] ) ), ]; // The following logic is unqiue to Vector (not used by legacy Vector) and // is planned to be moved in a follow-up patch. if ( !$this->isLegacy && $this->getSkin()->getUser()->isLoggedIn() ) { - $commonSkinData['data-navigation']['data-sidebar']['data-emphasized-sidebar-action'] = [ + $commonSkinData['data-sidebar']['data-emphasized-sidebar-action'] = [ 'href' => SpecialPage::getTitleFor( 'Preferences', false, diff --git a/includes/templates/Navigation.mustache b/includes/templates/Navigation.mustache deleted file mode 100644 index 9cdbe88..0000000 --- a/includes/templates/Navigation.mustache +++ /dev/null @@ -1,41 +0,0 @@ -{{! - string html-navigation-heading heading for entire navigation that is - usually hidden to screen readers - object data-personal-menu See PersonalMenu.mustache for documentation. - object data-namespace-tabs. See VectorTabs.mustache for documentation. - object data-variants. See VectorMenu.mustache for documentation. - object data-page-actions. See VectorTabs.mustache for documentation. - object data-page-actions-more. See VectorMenu.mustache for documentation. - object data-search-box. See SearchBox.mustache for documentation. - object data-sidebar. See Sidebar.mustache for documentation. -}} -
-

{{{html-navigation-heading}}}

-
- {{#data-personal-menu}} - {{>PersonalMenu}} - {{/data-personal-menu}} -
- {{#data-namespace-tabs}} - {{>VectorTabs}} - {{/data-namespace-tabs}} - {{#data-variants}} - {{>VectorMenu}} - {{/data-variants}} -
-
- {{#data-page-actions}} - {{>VectorTabs}} - {{/data-page-actions}} - {{#data-page-actions-more}} - {{>VectorMenu}} - {{/data-page-actions-more}} - {{#data-search-box}} - {{>SearchBox}} - {{/data-search-box}} -
-
- {{#data-sidebar}} - {{>Sidebar}} - {{/data-sidebar}} -
diff --git a/includes/templates/index.mustache b/includes/templates/index.mustache index 4093f3c..ac3d218 100644 --- a/includes/templates/index.mustache +++ b/includes/templates/index.mustache @@ -20,7 +20,15 @@ string html-catlinks string html-debuglog string html-dataAfterContent - object data-navigation for navigation template partial. see Navigation.mustache for documentation. + string html-navigation-heading heading for entire navigation that is + usually hidden to screen readers + object data-personal-menu See PersonalMenu.mustache for documentation. + object data-namespace-tabs. See VectorTabs.mustache for documentation. + object data-variants. See VectorMenu.mustache for documentation. + object data-page-actions. See VectorTabs.mustache for documentation. + object data-page-actions-more. See VectorMenu.mustache for documentation. + object data-search-box. See SearchBox.mustache for documentation. + object data-sidebar. See Sidebar.mustache for documentation. object data-footer for footer template partial. see Footer.mustache for documentation. string html-printtail HTML to render at the end of the page contained necessary script tags for ResourceLoader terminated with ``. @@ -57,9 +65,36 @@ {{{html-dataAfterContent}}} -{{#data-navigation}} -{{>Navigation}} -{{/data-navigation}} +
+

{{{html-navigation-heading}}}

+
+ {{#data-personal-menu}} + {{>PersonalMenu}} + {{/data-personal-menu}} +
+ {{#data-namespace-tabs}} + {{>VectorTabs}} + {{/data-namespace-tabs}} + {{#data-variants}} + {{>VectorMenu}} + {{/data-variants}} +
+
+ {{#data-page-actions}} + {{>VectorTabs}} + {{/data-page-actions}} + {{#data-page-actions-more}} + {{>VectorMenu}} + {{/data-page-actions-more}} + {{#data-search-box}} + {{>SearchBox}} + {{/data-search-box}} +
+
+ {{#data-sidebar}} + {{>Sidebar}} + {{/data-sidebar}} +
{{#data-footer}} {{>Footer}} {{/data-footer}} diff --git a/resources/skins.vector.styles/Navigation.less b/resources/skins.vector.styles/Navigation.less deleted file mode 100644 index 0100321..0000000 --- a/resources/skins.vector.styles/Navigation.less +++ /dev/null @@ -1,32 +0,0 @@ -@import 'mediawiki.mixins.less'; - -/* Hide, but keep accessible for screen-readers */ -#mw-navigation h2 { - position: absolute; - top: -9999px; -} - -#mw-head { - position: absolute; - top: 0; - right: 0; - width: 100%; -} - -/* Navigation Containers */ -#left-navigation { - float: left; - margin-left: 10em; - margin-top: 2.5em; - /* When right nav would overlap left nav, it's placed below it - (normal CSS floats behavior). This rule ensures that no empty space - is shown between them due to right nav's margin-top. Page layout - is still broken, but at least the nav overlaps only the page title - instead of half the content. */ - margin-bottom: -2.5em; -} - -#right-navigation { - float: right; - margin-top: 2.5em; -} diff --git a/resources/skins.vector.styles/common.less b/resources/skins.vector.styles/common.less index 8d5b0e3..9207f39 100644 --- a/resources/skins.vector.styles/common.less +++ b/resources/skins.vector.styles/common.less @@ -186,3 +186,34 @@ pre, margin-left: 10em; height: 5em; } + +/* Hide, but keep accessible for screen-readers */ +#mw-navigation h2 { + position: absolute; + top: -9999px; +} + +#mw-head { + position: absolute; + top: 0; + right: 0; + width: 100%; +} + +/* Navigation Containers */ +#left-navigation { + float: left; + margin-left: 10em; + margin-top: 2.5em; + /* When right nav would overlap left nav, it's placed below it + (normal CSS floats behavior). This rule ensures that no empty space + is shown between them due to right nav's margin-top. Page layout + is still broken, but at least the nav overlaps only the page title + instead of half the content. */ + margin-bottom: -2.5em; +} + +#right-navigation { + float: right; + margin-top: 2.5em; +} diff --git a/resources/skins.vector.styles/index.less b/resources/skins.vector.styles/index.less index ba865dc..87a5164 100644 --- a/resources/skins.vector.styles/index.less +++ b/resources/skins.vector.styles/index.less @@ -8,7 +8,6 @@ @import 'VectorTabs.less'; @import 'watchstar.less'; @import 'VectorMenu.less'; - @import 'Navigation.less'; @import 'Portal.less'; @import 'Sidebar.less'; @import 'Footer.less'; diff --git a/resources/skins.vector.styles/legacy.less b/resources/skins.vector.styles/legacy.less index 45c017e..d3e09bf 100644 --- a/resources/skins.vector.styles/legacy.less +++ b/resources/skins.vector.styles/legacy.less @@ -8,7 +8,6 @@ @import 'VectorTabs.less'; @import 'watchstar.less'; @import 'VectorMenu.less'; - @import 'Navigation.less'; @import 'Portal.less'; @import 'Sidebar.less'; @import 'Footer.less'; diff --git a/stories/Navigation.stories.data.js b/stories/Navigation.stories.data.js deleted file mode 100644 index 473279f..0000000 --- a/stories/Navigation.stories.data.js +++ /dev/null @@ -1,49 +0,0 @@ -import navTemplate from '!!raw-loader!../includes/templates/Navigation.mustache'; -import { PERSONAL_MENU_TEMPLATE_DATA, personalMenuTemplate } from './PersonalMenu.stories.data'; -import { pageActionsData, namespaceTabsData, vectorTabsTemplate } from './VectorTabs.stories.data'; -import { vectorMenuTemplate, moreData, variantsData } from './VectorMenu.stories.data'; -import { searchBoxData, searchBoxTemplate } from './SearchBox.stories.data'; -import { SIDEBAR_DATA, SIDEBAR_TEMPLATE_PARTIALS, sidebarTemplate } from './Sidebar.stories.data'; - -export const NAVIGATION_TEMPLATE_PARTIALS = Object.assign( {}, SIDEBAR_TEMPLATE_PARTIALS, { - SearchBox: searchBoxTemplate, - Sidebar: sidebarTemplate, - VectorTabs: vectorTabsTemplate, - VectorMenu: vectorMenuTemplate, - PersonalMenu: personalMenuTemplate -} ); - -export const NAVIGATION_TEMPLATE_DATA = { - loggedInWithVariantsAndOptOut: { - 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedOut, - 'data-namespace-tabs': namespaceTabsData, - 'data-page-actions': pageActionsData, - 'data-variants': variantsData, - 'data-search-box': searchBoxData, - 'data-sidebar': SIDEBAR_DATA.withPortalsAndOptOut, - 'html-navigation-heading': 'Navigation menu', - 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` - }, - loggedOutWithVariants: { - 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedOut, - 'data-namespace-tabs': namespaceTabsData, - 'data-page-actions': pageActionsData, - 'data-variants': variantsData, - 'data-search-box': searchBoxData, - 'data-sidebar': SIDEBAR_DATA.withPortals, - 'html-navigation-heading': 'Navigation menu', - 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` - }, - loggedInWithMoreActions: { - 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedInWithEcho, - 'data-namespace-tabs': namespaceTabsData, - 'data-page-actions': pageActionsData, - 'data-page-actions-more': moreData, - 'data-search-box': searchBoxData, - 'data-sidebar': SIDEBAR_DATA.withPortals, - 'html-navigation-heading': 'Navigation menu', - 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` - } -}; - -export { navTemplate }; diff --git a/stories/Navigation.stories.js b/stories/Navigation.stories.js deleted file mode 100644 index 9e07845..0000000 --- a/stories/Navigation.stories.js +++ /dev/null @@ -1,20 +0,0 @@ -import mustache from 'mustache'; -import { navTemplate, NAVIGATION_TEMPLATE_DATA, - NAVIGATION_TEMPLATE_PARTIALS } from './Navigation.stories.data'; -import '../.storybook/common.less'; -import '../resources/skins.vector.styles/Navigation.less'; -import '../resources/skins.vector.styles/watchstar.less'; - -export default { - title: 'Navigation (Header + Sidebar)' -}; - -export const navigationLoggedOutWithVariants = () => mustache.render( navTemplate, - NAVIGATION_TEMPLATE_DATA.loggedOutWithVariants, - NAVIGATION_TEMPLATE_PARTIALS -); - -export const navigationLoggedInWithMore = () => mustache.render( navTemplate, - NAVIGATION_TEMPLATE_DATA.loggedInWithMoreActions, - NAVIGATION_TEMPLATE_PARTIALS -); diff --git a/stories/skin.stories.js b/stories/skin.stories.js index 3945d45..bb8eb1d 100644 --- a/stories/skin.stories.js +++ b/stories/skin.stories.js @@ -4,11 +4,52 @@ import skinTemplate from '!!raw-loader!../includes/templates/index.mustache'; import { placeholder } from './utils'; import '../resources/skins.vector.styles/index.less'; -import { NAVIGATION_TEMPLATE_DATA, navTemplate, NAVIGATION_TEMPLATE_PARTIALS } from './Navigation.stories.data'; +import { PERSONAL_MENU_TEMPLATE_DATA, personalMenuTemplate } from './PersonalMenu.stories.data'; +import { pageActionsData, namespaceTabsData, vectorTabsTemplate } from './VectorTabs.stories.data'; +import { vectorMenuTemplate, moreData, variantsData } from './VectorMenu.stories.data'; +import { searchBoxData, searchBoxTemplate } from './SearchBox.stories.data'; +import { SIDEBAR_DATA, SIDEBAR_TEMPLATE_PARTIALS, sidebarTemplate } from './Sidebar.stories.data'; import { FOOTER_TEMPLATE_DATA, footerTemplate } from './Footer.stories.data'; -const TEMPLATE_PARTIALS = Object.assign( {}, NAVIGATION_TEMPLATE_PARTIALS, { - Navigation: navTemplate, +const NAVIGATION_TEMPLATE_DATA = { + loggedInWithVariantsAndOptOut: { + 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedInWithEcho, + 'data-namespace-tabs': namespaceTabsData, + 'data-page-actions': pageActionsData, + 'data-variants': variantsData, + 'data-search-box': searchBoxData, + 'data-sidebar': SIDEBAR_DATA.withPortalsAndOptOut, + 'html-navigation-heading': 'Navigation menu', + 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` + }, + loggedOutWithVariants: { + 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedOut, + 'data-namespace-tabs': namespaceTabsData, + 'data-page-actions': pageActionsData, + 'data-variants': variantsData, + 'data-search-box': searchBoxData, + 'data-sidebar': SIDEBAR_DATA.withPortals, + 'html-navigation-heading': 'Navigation menu', + 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` + }, + loggedInWithMoreActions: { + 'data-personal-menu': PERSONAL_MENU_TEMPLATE_DATA.loggedInWithEcho, + 'data-namespace-tabs': namespaceTabsData, + 'data-page-actions': pageActionsData, + 'data-page-actions-more': moreData, + 'data-search-box': searchBoxData, + 'data-sidebar': SIDEBAR_DATA.withPortals, + 'html-navigation-heading': 'Navigation menu', + 'html-logo-attributes': `class="mw-wiki-logo" href="/wiki/Main_Page" title="Visit the main page"` + } +}; + +const TEMPLATE_PARTIALS = Object.assign( {}, SIDEBAR_TEMPLATE_PARTIALS, { + SearchBox: searchBoxTemplate, + Sidebar: sidebarTemplate, + VectorTabs: vectorTabsTemplate, + VectorMenu: vectorMenuTemplate, + PersonalMenu: personalMenuTemplate, Footer: footerTemplate } ); @@ -35,14 +76,13 @@ const HTML_INDICATORS = `
`; -export const vectorLegacyLoggedOut = () => mustache.render( skinTemplate, { +export const vectorLegacyLoggedOut = () => mustache.render( skinTemplate, Object.assign( { 'html-title': 'Vector 2019', 'page-isarticle': true, 'msg-tagline': 'From Wikipedia, the free encyclopedia', 'html-userlangattributes': htmluserlangattributes, 'msg-jumptonavigation': 'Jump to navigation', 'msg-jumptosearch': 'Jump to search', - 'data-navigation': NAVIGATION_TEMPLATE_DATA.loggedOutWithVariants, // site specific 'data-footer': FOOTER_TEMPLATE_DATA, @@ -58,16 +98,15 @@ export const vectorLegacyLoggedOut = () => mustache.render( skinTemplate, { 'html-dataAfterContent': placeholder( 'Extensions can add here e.g. Related Articles.', 100 ), 'html-indicators': HTML_INDICATORS, 'html-subtitle': placeholder( 'Extensions can configure subtitle', 20 ) -}, TEMPLATE_PARTIALS ); +}, NAVIGATION_TEMPLATE_DATA.loggedOutWithVariants ), TEMPLATE_PARTIALS ); -export const vectorLegacyLoggedIn = () => mustache.render( skinTemplate, { +export const vectorLegacyLoggedIn = () => mustache.render( skinTemplate, Object.assign( { 'html-title': 'Vector 2019', 'page-isarticle': true, 'msg-tagline': 'From Wikipedia, the free encyclopedia', 'html-userlangattributes': htmluserlangattributes, 'msg-jumptonavigation': 'Jump to navigation', 'msg-jumptosearch': 'Jump to search', - 'data-navigation': NAVIGATION_TEMPLATE_DATA.loggedInWithMoreActions, // site specific 'data-footer': FOOTER_TEMPLATE_DATA, @@ -77,16 +116,15 @@ export const vectorLegacyLoggedIn = () => mustache.render( skinTemplate, { 'html-bodycontent': placeholder( 'Article content goes here' ), 'html-printfooter': `Retrieved from ‘https://en.wikipedia.org/w/index.php?title=this&oldid=blah’`, 'html-catlinks': placeholder( 'Category links component from mediawiki core', 50 ) -}, TEMPLATE_PARTIALS ); +}, NAVIGATION_TEMPLATE_DATA.loggedInWithMoreActions ), TEMPLATE_PARTIALS ); -export const vectorLoggedIn = () => mustache.render( skinTemplate, { +export const vectorLoggedIn = () => mustache.render( skinTemplate, Object.assign( { 'html-title': 'Vector 2020', 'page-isarticle': true, 'msg-tagline': 'From Wikipedia, the free encyclopedia', 'html-userlangattributes': htmluserlangattributes, 'msg-jumptonavigation': 'Jump to navigation', 'msg-jumptosearch': 'Jump to search', - 'data-navigation': NAVIGATION_TEMPLATE_DATA.loggedInWithVariantsAndOptOut, // site specific 'data-footer': FOOTER_TEMPLATE_DATA, @@ -96,4 +134,4 @@ export const vectorLoggedIn = () => mustache.render( skinTemplate, { 'html-bodycontent': placeholder( 'Article content goes here' ), 'html-printfooter': `Retrieved from ‘https://en.wikipedia.org/w/index.php?title=this&oldid=blah’`, 'html-catlinks': placeholder( 'Category links component from mediawiki core', 50 ) -}, TEMPLATE_PARTIALS ); +}, NAVIGATION_TEMPLATE_DATA.loggedInWithVariantsAndOptOut ), TEMPLATE_PARTIALS );