From 78787d96657298aa6d2a419700203004d5f8e05c Mon Sep 17 00:00:00 2001 From: Nicholas Ray Date: Tue, 8 Sep 2020 17:20:58 -0600 Subject: [PATCH] Switch to navigation-first DOM order under `$wgVectorIsSearchInHeader` feature flag This moves the header, navigation, sidebar, and article toolbar to be before the content in the DOM. As a result, a lot of absolute positioning logic can be removed and styles can be simplified. Note that although the sidebar was moved from the header into the workspace container allowing it to de-absolutely positioned, its absolute positioning was kept intact as it has a fair amount of complexity that should be handled in a separate task. To activate, set `$wgVectorIsSearchInHeader = true;` Changes that could cause concern: * The "jump to search" link was removed as the search is now much earlier in the DOM and I questioned the value of keeping this. However, it can be added back in if this change is contentious. * A "jump to content" link was added to account for the new DOM order. * Because the sidebar was taken out of the header, users will not be able to tab from the sidebar button into the sidebar without additional tweaking (e.g. should we add JS to enable this?). It was deemed that this work can be saved as a follow-up task. * I applied `overflow-y: auto` to the `mw-page-container` because the header's top margin was collapsing and caused whitespace to appear between the viewport and the header. Alternatively, we could apply a top padding to the page container and remove the header's top margin. I went for the simplest solution but am open to alternatives. * I left the footer as-is in this patch to minimize risk. It might be cleaner later on to move the footer inside the workspace container which would leave only one workspace container. Bug: T261802 Change-Id: Ic553fab3bde25769b103d899b92b3b694c00c384 --- bundlesize.config.json | 2 +- i18n/en.json | 1 + i18n/qqq.json | 1 + includes/templates/Header.mustache | 22 +++++++ includes/templates/Navigation.mustache | 21 +++++++ includes/templates/skin.mustache | 62 ++++++------------- resources/skins.vector.styles/Sidebar.less | 3 +- .../skins.vector.styles/layout-default.less | 39 ++++++++---- .../skins.vector.styles/layout-max-width.less | 45 ++++++++++---- .../layout-search-header.less | 41 ------------ skin.json | 1 + 11 files changed, 131 insertions(+), 107 deletions(-) create mode 100644 includes/templates/Header.mustache create mode 100644 includes/templates/Navigation.mustache diff --git a/bundlesize.config.json b/bundlesize.config.json index 5c873b5..95b05fa 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -5,7 +5,7 @@ }, { "resourceModule": "skins.vector.styles", - "maxSize": "9.1 kB" + "maxSize": "9.2 kB" }, { "resourceModule": "skins.vector.styles.responsive", diff --git a/i18n/en.json b/i18n/en.json index e45a7fe..adec5ab 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -24,6 +24,7 @@ "vector-view-viewsource": "View source", "vector-jumptonavigation": "Jump to navigation", "vector-jumptosearch": "Jump to search", + "vector-jumptocontent": "Jump to content", "vector-more-actions": "More", "vector-search-loader": "Loading search suggestions" } diff --git a/i18n/qqq.json b/i18n/qqq.json index b6c1ec4..fb57216 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -35,6 +35,7 @@ "vector-view-viewsource": "Tab label in the Vector skin.\n{{Identical|View source}}", "vector-jumptonavigation": "Accessibility link for jumping to the navigation links. Visually hidden by default.\n\nSee also\n* {{msg-mw|Navigation}}", "vector-jumptosearch": "Accessibility link for jumping to the site search. Visually hidden by default.\n\nSee also\n* {{msg-mw|Search}}", + "vector-jumptocontent": "Accessibility link for jumping to the content and skipping the navigation. Visually hidden by default.", "vector-more-actions": "Label in the Vector skin's menu for the less-important or rarer actions which are not shown as tabs (like moving the page, or for sysops deleting or protecting the page), as well as (for users with a narrow viewing window in their browser) the less-important tab actions which the user's browser is unable to fit in. {{Identical|More}}", "vector-search-loader": "Text to display below search input while the search suggestion module is loading" } diff --git a/includes/templates/Header.mustache b/includes/templates/Header.mustache new file mode 100644 index 0000000..5eeb10c --- /dev/null +++ b/includes/templates/Header.mustache @@ -0,0 +1,22 @@ +
+ + {{^is-search-in-header}} +
+ {{#data-sidebar}}{{>Sidebar}}{{/data-sidebar}} +
+ {{/is-search-in-header}} + {{>Logo}} + {{#is-search-in-header}} + {{#data-search-box}}{{>SearchBox}}{{/data-search-box}} + {{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}} + {{/is-search-in-header}} +
diff --git a/includes/templates/Navigation.mustache b/includes/templates/Navigation.mustache new file mode 100644 index 0000000..b437ec4 --- /dev/null +++ b/includes/templates/Navigation.mustache @@ -0,0 +1,21 @@ +
+

{{msg-navigation-heading}}

+
+ {{^is-search-in-header}} + {{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}} + {{/is-search-in-header}} +
+
+ {{#data-namespace-tabs}}{{>Menu}}{{/data-namespace-tabs}} + {{#data-variants}}{{>Menu}}{{/data-variants}} +
+
+ {{#data-page-actions}}{{>Menu}}{{/data-page-actions}} + {{#data-page-actions-more}}{{>Menu}}{{/data-page-actions-more}} + {{^is-search-in-header}} + {{#data-search-box}}{{>SearchBox}}{{/data-search-box}} + {{/is-search-in-header}} +
+
+
+
diff --git a/includes/templates/skin.mustache b/includes/templates/skin.mustache index b9d0928..f74032c 100644 --- a/includes/templates/skin.mustache +++ b/includes/templates/skin.mustache @@ -12,6 +12,7 @@ string html-newtalk string msg-vector-jumptonavigation string msg-vector-jumptosearch + string msg-vector-jumptocontent string html-body-content string html-categories string html-after-content @@ -30,9 +31,14 @@ object data-footer for footer template partial. see Footer.mustache for documentation. }}
+ {{#is-search-in-header}} + {{msg-vector-jumptocontent}} + {{/is-search-in-header}}
+{{^is-search-in-header}}
+{{/is-search-in-header}} +{{#is-search-in-header}} +{{>Header}} +{{/is-search-in-header}} +
+ {{#is-search-in-header}} + {{#data-sidebar}}{{>Sidebar}}{{/data-sidebar}} + {{>Navigation}} + {{/is-search-in-header}}
{{! `role` is unnecessary but kept to support selectors in any gadgets or user styles. }} @@ -59,8 +73,10 @@ using this place to insert extra elements before. }}
+ {{^is-search-in-header}} {{msg-vector-jumptonavigation}} {{msg-vector-jumptosearch}} + {{/is-search-in-header}} {{{html-body-content}}} {{{html-categories}}}
@@ -69,48 +85,10 @@
{{! END mw-content-container }}
{{! END mw-workspace-container }} -
- -
- {{#data-sidebar}}{{>Sidebar}}{{/data-sidebar}} -
- {{>Logo}} - {{#is-search-in-header}} - {{#data-search-box}}{{>SearchBox}}{{/data-search-box}} - {{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}} - {{/is-search-in-header}} -
- -
-

{{msg-navigation-heading}}

-
- {{^is-search-in-header}} - {{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}} - {{/is-search-in-header}} -
-
- {{#data-namespace-tabs}}{{>Menu}}{{/data-namespace-tabs}} - {{#data-variants}}{{>Menu}}{{/data-variants}} -
-
- {{#data-page-actions}}{{>Menu}}{{/data-page-actions}} - {{#data-page-actions-more}}{{>Menu}}{{/data-page-actions-more}} - {{^is-search-in-header}} - {{#data-search-box}}{{>SearchBox}}{{/data-search-box}} - {{/is-search-in-header}} -
-
-
-
+{{^is-search-in-header}} +{{>Header}} +{{>Navigation}} +{{/is-search-in-header}}
diff --git a/resources/skins.vector.styles/Sidebar.less b/resources/skins.vector.styles/Sidebar.less index 3d4cb3e..8b3e3a5 100644 --- a/resources/skins.vector.styles/Sidebar.less +++ b/resources/skins.vector.styles/Sidebar.less @@ -61,7 +61,8 @@ // Use the MediaWiki checkbox hack class from checkboxHack.less. This class exists on the // checkbox input for the menu panel. -#mw-sidebar-checkbox:not( :checked ) ~ .mw-header .mw-sidebar { +.skin-vector-search-header-legacy #mw-sidebar-checkbox:not( :checked ) ~ .mw-header .mw-sidebar, +.skin-vector-search-header #mw-sidebar-checkbox:not( :checked ) ~ .mw-workspace-container .mw-sidebar { // Turn off presentation so that screen readers get the same effect as visually hiding. // Visibility and opacity can be animated. If animation is unnecessary, use `display: none` // instead to avoid hidden rendering. diff --git a/resources/skins.vector.styles/layout-default.less b/resources/skins.vector.styles/layout-default.less index f8c1117..1fb063e 100644 --- a/resources/skins.vector.styles/layout-default.less +++ b/resources/skins.vector.styles/layout-default.less @@ -33,19 +33,22 @@ body { } /* Space for header above content */ -.mw-header-placeholder { +.skin-vector-search-header-legacy .mw-header-placeholder { // Reserve space for the absolute positioned header and tabs. height: @height-header + @height-tabs; } /* Header layout */ -.mw-header { +.skin-vector-search-header-legacy .mw-header { position: absolute; top: 0; left: 0; right: 0; - // A height is set to account for projects where no icon is set. - height: @height-logo-icon; +} + +.mw-header { + // A min-height is set to account for projects where no icon is set. + min-height: @height-logo-icon; margin: @margin-top-header 0 @margin-bottom-header; padding: @padding-vertical-header @padding-horizontal-header; // Vertical centering of header elements (IE>=11), requires flex. @@ -94,7 +97,7 @@ body { } /* Tabs */ -#mw-head { +.skin-vector-search-header-legacy #mw-head { position: absolute; top: 0; right: 0; @@ -102,8 +105,12 @@ body { } /* Navigation Containers */ -#left-navigation { - float: left; +.mw-article-toolbar-container { + // Clear the floats on #left-navigation and #right-navigation. + .mixin-clearfix(); +} + +.skin-vector-search-header-legacy #left-navigation { margin-top: @height-header; /* When right nav would overlap left nav, it's placed below it (normal CSS floats behavior). This rule ensures that no empty space @@ -113,9 +120,16 @@ body { margin-bottom: -@height-header; } +#left-navigation { + float: left; +} + +.skin-vector-search-header-legacy #right-navigation { + margin-top: @height-header; +} + #right-navigation { float: right; - margin-top: @height-header; } .skin-vector-search-header-legacy #p-personal { @@ -143,12 +157,15 @@ body { margin-right: @margin-horizontal-sidebar-button-icon; // Accidentally the same. } -#mw-panel { - position: absolute; +.skin-vector-search-header-legacy #mw-panel { // The sidebar is absolutely positioned inside the header which applies a top // margin so we need to subtract this top margin in order to get the correct // sidebar position. top: @height-header - @margin-top-header; +} + +#mw-panel { + position: absolute; left: 0; width: @width-grid-column-one; .box-sizing( border-box ); @@ -157,6 +174,6 @@ body { z-index: @z-index-sidebar; } -.mw-footer { +.skin-vector-search-header-legacy .mw-footer { margin-top: 0; } diff --git a/resources/skins.vector.styles/layout-max-width.less b/resources/skins.vector.styles/layout-max-width.less index 3ee86b0..60c9aa9 100644 --- a/resources/skins.vector.styles/layout-max-width.less +++ b/resources/skins.vector.styles/layout-max-width.less @@ -56,26 +56,26 @@ margin-left: 0; } - #mw-head { + &.skin-vector-search-header-legacy #mw-head { width: auto; left: 0; right: 0; } - #left-navigation { + &.skin-vector-search-header-legacy #left-navigation { margin-top: 0; margin-bottom: 0; } - #right-navigation { + &.skin-vector-search-header-legacy #right-navigation { margin-top: 0; } - #p-personal { + &.skin-vector-search-header-legacy #p-personal { right: 0; } - #p-search { + &.skin-vector-search-header-legacy #p-search { margin-right: 0; } @@ -117,6 +117,13 @@ background-color: @background-color-page-container; } + &.skin-vector-search-header .mw-page-container { + // Establish a new block formatting context to prevent header top margin + // collapsing and causing whitespace to appear between the header and + // viewport. + overflow-y: auto; + } + // Used as a container for absolutely positioned elements. .mw-page-container-inner { position: relative; @@ -128,16 +135,30 @@ margin-right: auto; } + &.skin-vector-search-header .mw-workspace-container { + // The sidebar is absolutely positioned relative to the + // mw-workspace-container. + position: relative; + } + .mw-content-container { max-width: @max-width-content-container; margin-left: auto; margin-right: auto; } - .mw-article-toolbar-container { + &.skin-vector-search-header-legacy .mw-article-toolbar-container { margin-top: @height-header; - // Clear the floats on #left-navigation and #right-navigation. - .mixin-clearfix(); + } + + &.skin-vector-search-header .mw-article-toolbar-container { + // We want to keep the max-width of the article-toolbar-container the + // same max-width as the article page's content container in order to + // prevent it from moving when going from an article page to a + // history/special page. + max-width: @max-width-content-container; + margin-left: auto; + margin-right: auto; } .mw-sidebar-container { @@ -165,7 +186,7 @@ // same max-width as the article page's content container in order to // prevent it from moving when going from an article page to a // history/special page. - .mw-article-toolbar-container { + &.skin-vector-search-header-legacy .mw-article-toolbar-container { max-width: @max-width-content-container; } @@ -180,7 +201,8 @@ // `.mw-page-container` because that will cut off the sidebar. Therefore, we // calculate the maximum distance from the start of `mw-page-container` to the // start of the sidebar. - #mw-sidebar-checkbox:not( :checked ) ~ .mw-header .mw-sidebar { + &.skin-vector-search-header-legacy #mw-sidebar-checkbox:not( :checked ) ~ .mw-header .mw-sidebar, + &.skin-vector-search-header #mw-sidebar-checkbox:not( :checked ) ~ .mw-workspace-container .mw-sidebar { .transform( translateX( -( @max-width-page-container - @max-width-workspace-container ) / 2 ) ); } @@ -197,7 +219,8 @@ @media ( max-width: @max-width-margin-start-content ) { // Adjusts the content and mw-article-toolbar-container. .mw-checkbox-hack-checkbox:checked ~ .mw-workspace-container .mw-content-container, - .mw-checkbox-hack-checkbox:checked ~ #mw-navigation .mw-content-container { + &.skin-vector-search-header-legacy .mw-checkbox-hack-checkbox:checked ~ #mw-navigation .mw-content-container, + &.skin-vector-search-header .mw-checkbox-hack-checkbox:checked ~ .mw-workspace-container .mw-article-toolbar-container { margin-left: @margin-start-content; } diff --git a/resources/skins.vector.styles/layout-search-header.less b/resources/skins.vector.styles/layout-search-header.less index 7259d6b..d1d903b 100644 --- a/resources/skins.vector.styles/layout-search-header.less +++ b/resources/skins.vector.styles/layout-search-header.less @@ -54,47 +54,6 @@ // Support IE9: This is reset in @support query below if Flexbox is available. float: right; } - - // If we don't have the space adjust header. - // Increase height and margins to accommodate personal tools underneath. - @media ( max-width: @width-comfortable ) { - @height-header-adjusted: @height-header + @height-personal-tools; - - #mw-panel { - top: @height-header-adjusted - @margin-top-header; - } - - .mw-header-placeholder { - height: @height-header-adjusted + @height-tabs; - } - - #left-navigation { - margin-top: @height-header-adjusted; - margin-bottom: 0; - } - - #right-navigation { - margin-top: -@height-tabs; - clear: left; - } - } -} - -// when max-width is also enabled... -.skin-vector-search-header.skin-vector-max-width { - #left-navigation { - margin-top: 0; - } - - .mw-article-toolbar-container { - margin-top: @height-header + @height-personal-tools; - } - - @media ( min-width: @width-comfortable ) { - .mw-article-toolbar-container { - margin-top: @height-header; - } - } } // If flexbox supported reset the floats that are supporting IE9 diff --git a/skin.json b/skin.json index fce0d74..09c032d 100644 --- a/skin.json +++ b/skin.json @@ -35,6 +35,7 @@ "vector-action-toggle-sidebar", "vector-jumptonavigation", "vector-jumptosearch", + "vector-jumptocontent", "sitesubtitle", "sitetitle", "tagline"