Hygiene: extract ToggleList to a reusable component

A list that toggles visibility via the checkbox hack is needed in at
least two spots: the page actions overflow menu and the user menu. This
patch makes several refactors to turn what was previously hardcoded into
page actions a reusable component:

- Start a new components directory. Components are reusable and
  composable. The subdirectories are organized by function, not
  ResourceLoader module bundling which greatly improves the ability to
  see a component's full functionality in one directory instead of
  examining the entire codebase. See updates to README.

- Extract pageactions.less into:
  - ToggleList.less: LESS for any checkbox hack list.
  - DropDownList.less: LESS for lists that open downwards.
  - MenuListItem.less: LESS for list items of menus.
  The division makes it easier to see concerns, dependencies, and change
  code.

- Move pageActionMenu.mustache to a component and extract ToggleList
  template.

- Extract ToggleList.js from Toolbar.js.

Bug: T214540
Change-Id: I171831469a6733c458bc5c7ba249a5096ca975b8
This commit is contained in:
Stephen Niedzielski 2019-06-20 13:17:50 -06:00
parent e10f8a8b16
commit edb4385345
20 changed files with 344 additions and 196 deletions

View File

@ -243,3 +243,54 @@ Defines the sampling rate for the MobileWebMainMenuClickTracking schema.
* Type: `Number`
* Default: `0`
### Components
Components may be shared between server and client. Keeping all code for a single component only in
one directory makes it easier to understand the complete domain of a component, all of its implicit
dependencies, and also what it is independent of. The structure does not hint at ResourceLoader
module bundling of resources and code. That is the domain of skin.json.
New components are stored under components/. Potential older components are stored under includes/
and resources/, and those directory structures imperfectly represent ResourceLoader module
divisions.
#### Mustache
Mustache templates at the root components/ directory, like components/PageActionsMenu.mustache or
components/ToggleList.mustache, are designed to be rendered as root templates not partials. E.g.:
```lang=php
// 🆗
$templatesDir = __DIR__ . '/../../components';
$invalidateTemplateCache = false;
$templateParser = new TemplateParser( $templatesDir, $invalidateTemplateCache );
// Render components/ToggleList.mustache not components/ToggleList/ToggleList.mustache.
$html = $templateParser->processTemplate( 'ToggleList', $data );
```
Attempting to render a partial as a template root will fail because of components/ root path
assumptions:
```lang=php
// 🚫
$templatesDir = __DIR__ . '/../../components/ToggleList';
$invalidateTemplateCache = false;
$templateParser = new TemplateParser( $templatesDir, $invalidateTemplateCache );
// Error: components/ToggleList/ToggleList.mustache references
// components/ToggleList/ToggleList/ToggleListItem.mustache which does not exist.
$html = $templateParser->processTemplate( 'ToggleList', $data );
```
Partials in components/ subdirectories, like components/PageActionsMenu/PageActionsMenu.mustache or
components/ToggleList/ToggleList.mustache, are for in-template partial composition only as their
paths assume the render root is components/. E.g.:
```lang=mustache
{{! Include components/ToggleList/ToggleList.mustache, not components/ToggleList.mustache. }}
{{> ToggleList/ToggleList}}
```

View File

@ -0,0 +1 @@
{{> PageActionsMenu/PageActionsMenu}}

View File

@ -0,0 +1,18 @@
<nav class="page-actions-menu">
<ul id="page-actions" class="page-actions-menu__list">
{{#toolbar}}
<li id="{{name}}" class="page-actions-menu__list-item">
{{#components}}
<a id="{{id}}" href="{{href}}" class="{{class}}" data-event-name="{{data-event-name}}" role="button" title="{{title}}">
{{text}}
</a>
{{/components}}
</li>
{{/toolbar}}
{{#overflowMenu}}
<li id="{{item-id}}" class="page-actions-menu__list-item">
{{> ToggleList/ToggleList}}
</li>
{{/overflowMenu}}
</ul>
</nav>

View File

@ -0,0 +1 @@
{{> ToggleList/ToggleList}}

View File

@ -0,0 +1,22 @@
// A DropDownList is a ToggleList that extends downward.
@import '../../minerva.less/minerva.mixins';
.toggle-list__list--drop-down {
transform: translateY( -8px );
// Animate menu visibility, opacity, and translation changes in and out. Visibility must be
// animated since it's a boolean and nothing can be seen in display hidden. Visibility itself
// cannot be animated as it causes a flash on page load in Chromium due to
// https://bugs.chromium.org/p/chromium/issues/detail?id=332189. The effect is that the menu is
// animated in but not animated out.
.transition( opacity .1s ease-in-out, -webkit-tap-highlight-color 0s ease-in-out, transform .1s ease-in-out; );
// When cursor is pointer and -webkit-tap-highlight-color is set, the color does not seem to
// transition. Clear it.
-webkit-tap-highlight-color: transparent;
}
.toggle-list__checkbox:checked ~ .toggle-list__list--drop-down {
transform: translateY( 0 );
}

View File

@ -0,0 +1,32 @@
// A MenuListItem is a ToggleList item for menus.
@import '../../minerva.less/minerva.variables';
@import '../../minerva.less/minerva.mixins';
.toggle-list-item__anchor--menu {
font-size: @pageActionFontSize;
font-weight: bold;
// Fill the list item cell.
.box-sizing( border-box );
display: block;
width: 100%;
//
padding: 1em;
white-space: nowrap;
// Left-align text. Button elements are centered.
text-align: left;
//
color: @grayMediumDark;
&:visited, &:active {
// Visited and active links need extra specificity.
color: @grayMediumDark;
}
//
// Make the app feel like an app, not a JPEG. When hovering over a menu item, add a little
// interactivity.
&:hover {
text-decoration: none;
background: @grayLightest;
}
}

View File

@ -0,0 +1,99 @@
( function ( M ) {
var
/** The component selector. */
selector = '.toggle-list',
/** The visible label icon associated with the checkbox. */
toggleSelector = '.toggle-list__toggle',
/** The underlying hidden checkbox that controls list visibility. */
checkboxSelector = '.toggle-list__checkbox',
listSelector = '.toggle-list__list';
/**
* Automatically dismiss the list when clicking or focusing elsewhere and update the
* aria-expanded attribute based on list visibility.
* @param {Window} window
* @param {HTMLElement} component
* @param {OO.EventEmitter} eventBus
* @param {boolean} [resize] If true, resize the menu on scroll and window resize.
* @return {void}
*/
function bind( window, component, eventBus, resize ) {
var
toggle = component.querySelector( toggleSelector ),
checkbox = /** @type {HTMLInputElement} */ (
component.querySelector( checkboxSelector )
),
list = component.querySelector( listSelector );
window.addEventListener( 'click', function ( event ) {
if ( event.target !== toggle && event.target !== checkbox ) {
// Something besides the button or checkbox was tapped. Dismiss the list.
_dismiss( checkbox );
}
}, true );
// If focus is given to any element outside the list, dismiss the list. Setting a focusout
// listener on list would be preferable, but this interferes with the click listener.
window.addEventListener( 'focusin', function ( event ) {
if ( event.target instanceof Node && !component.contains( event.target ) ) {
// Something besides the button or checkbox was focused. Dismiss the list.
_dismiss( checkbox );
}
}, true );
checkbox.addEventListener( 'change', _updateAriaExpanded.bind( undefined, checkbox ) );
if ( resize ) {
eventBus.on( 'scroll:throttled', _resize.bind( undefined, list ) );
eventBus.on( 'resize:throttled', _resize.bind( undefined, list ) );
}
}
/**
* @param {HTMLElement} component
* @param {boolean} [resize] If true, resize the menu to fit within the window.
* @return {void}
*/
function render( component, resize ) {
var list = /** @type {HTMLElement} */ ( component.querySelector( listSelector ) );
if ( resize ) {
_resize( list );
}
}
/**
* Hides the list.
* @param {HTMLInputElement} checkbox
* @return {void}
*/
function _dismiss( checkbox ) {
checkbox.checked = false;
_updateAriaExpanded( checkbox );
}
/**
* @param {HTMLElement} list
* @return {void}
*/
function _resize( list ) {
var rect = list.getClientRects()[ 0 ];
if ( rect ) {
list.style.maxHeight = window.document.documentElement.clientHeight - rect.top + 'px';
}
}
/**
* Revise the aria-expanded state to match the checked state.
* @param {HTMLInputElement} checkbox
* @return {void}
*/
function _updateAriaExpanded( checkbox ) {
checkbox.setAttribute( 'aria-expanded', ( !!checkbox.checked ).toString() );
}
M.define( 'skins.minerva.scripts/ToggleList', Object.freeze( {
selector: selector,
render: render,
bind: bind
} ) );
}( mw.mobileFrontend ) );

View File

@ -0,0 +1,51 @@
@import '../../minerva.less/minerva.variables';
.toggle-list__checkbox {
// Always occlude the checkbox. The checkbox display cannot be none since its focus state is used
// for other selectors.
position: absolute;
z-index: @z-indexOccluded;
opacity: 0;
}
.toggle-list__toggle {
// Use the hand icon for the toggle button which is actually a checkbox label.
cursor: pointer;
}
.toggle-list__checkbox:focus + .toggle-list__toggle {
// The toggle button / label itself cannot receive focus but the underlying checkbox can. Keep
// the button and checkbox focus presentation in sync. From
// resources/src/mediawiki.toc.styles/screen.less.
outline: dotted 1px; /* Firefox style for focus */
outline: auto @colorProgressiveHighlight; /* Webkit style for focus */
}
.touch-events .toggle-list__checkbox:focus + .toggle-list__toggle {
// Buttons have no focus outline on mobile.
outline: 0;
}
.toggle-list__list {
// The menu appears over the content and occupies no room within it.
position: absolute;
//
// If max-height is set and the height exceeds it, add a vertical scrollbar.
overflow-y: auto;
//
// The menu floats over content but below overlays.
z-index: @z-indexDrawer;
//
background: @skinContentBgColor;
box-shadow: 0 5px 17px 0 rgba( 0, 0, 0, 0.24 ), 0 0 1px @colorGray10;
border-radius: @borderRadius;
//
visibility: hidden;
opacity: 0;
}
.toggle-list__checkbox:checked ~ .toggle-list__list {
// Reveal the list when checked.
visibility: visible;
opacity: 1;
}

View File

@ -0,0 +1,27 @@
{{!
A list with visibility toggled by a checkbox.
string|null class Optional CSS class for the root element.
string checkboxID CSS identifier unique to the page needed to connect label and input.
string|null toggleID Optional toggle button CSS identifier to connect label and toggle aria.
string|null toggleClass Optional toggle button CSS class.
string|null listClass Optional list CSS class.
string|null text Optional text and aria label for the toggle button.
array|null items Optional array of drop down list items for the unordered list.
}}
<div class="toggle-list {{class}}">
<input
type="checkbox"
id="{{checkboxID}}"
class="toggle-list__checkbox"
role="button"
aria-labelledby="{{toggleID}}"
aria-expanded="false">
<label id="{{toggleID}}" class="toggle-list__toggle {{toggleClass}}" for="{{checkboxID}}">
{{text}}
</label>
<ul class="toggle-list__list {{listClass}}">
{{#items}}
{{> ToggleList/ToggleListItem}}
{{/items}}
</ul>
</div>

View File

@ -0,0 +1,11 @@
{{!
array components
string|null components.class Optional anchor CSS class.
string|null components.href Optional URI.
string|null components.text Optional text.
}}
{{#components}}
<li class="toggle-list-item">
<a class="toggle-list-item__anchor {{class}}" href="{{href}}">{{text}}</a>
</li>
{{/components}}

View File

@ -77,7 +77,9 @@ class DefaultOverflowBuilder implements IOverflowBuilder {
new PageActionMenuEntry(
'page-actions-overflow-' . $name,
$href,
MinervaUI::iconClass( '', 'before', 'wikimedia-ui-' . $icon . '-base20' ),
MinervaUI::iconClass(
'', 'before', 'wikimedia-ui-' . $icon . '-base20 toggle-list-item__anchor--menu'
),
$this->messageLocalizer->msg( 'minerva-page-actions-' . $name )
) : null;
}

View File

@ -73,11 +73,15 @@ final class PageActionsDirector {
'toolbar' => $toolbar->getEntries()
];
if ( $overflowMenu->hasEntries() ) {
// See components/ToggleList.
$menu[ 'overflowMenu' ] = [
'item-id' => 'page-actions-overflow',
'class' => MinervaUI::iconClass( 'page-actions-overflow' ),
'checkboxID' => 'page-actions-overflow-checkbox',
'toggleID' => 'page-actions-overflow-toggle',
'toggleClass' => MinervaUI::iconClass( 'page-actions-overflow' ),
'listClass' => 'page-actions-overflow-list toggle-list__list--drop-down',
'text' => $this->messageLocalizer->msg( 'minerva-page-actions-overflow' ),
'pageActions' => $overflowMenu->getEntries()
'items' => $overflowMenu->getEntries()
];
}
return $menu;

View File

@ -102,7 +102,9 @@ class UserNamespaceOverflowBuilder implements IOverflowBuilder {
new PageActionMenuEntry(
'page-actions-overflow-' . $name,
$href,
MinervaUI::iconClass( '', 'before', 'wikimedia-ui-' . $icon . '-base20' ),
MinervaUI::iconClass(
'', 'before', 'wikimedia-ui-' . $icon . '-base20 toggle-list-item__anchor--menu'
),
$this->messageLocalizer->msg( 'minerva-page-actions-' . $name )
) : null;
}

View File

@ -97,12 +97,12 @@ class MinervaTemplate extends BaseTemplate {
* @return string
*/
protected function getPageActionsHtml() {
$templateParser = new TemplateParser( __DIR__ );
$templateParser = new TemplateParser( __DIR__ . '/../../components' );
$pageActions = $this->getPageActions();
$html = '';
if ( $pageActions && $pageActions['toolbar'] ) {
$html = $templateParser->processTemplate( 'pageActionMenu', $pageActions );
$html = $templateParser->processTemplate( 'PageActionsMenu', $pageActions );
}
return $html;
}

View File

@ -15,8 +15,8 @@
</div>
<div class="search-box">
<input class="search skin-minerva-search-trigger" type="search" name="search" id="searchInput"
autocomplete="off" placeholder="{{placeholder}}" aria-label="{{placeholder}}"
value="{{search}}">
autocomplete="off" placeholder="{{placeholder}}" aria-label="{{placeholder}}"
value="{{search}}">
</div>
<div>{{{searchButton}}}</div>
{{^isAnon}}<div>{{#userNotificationsData}}{{>userNotifications}}{{/userNotificationsData}}</div>{{/isAnon}}

View File

@ -1,32 +0,0 @@
<nav class="page-actions-menu">
<ul id="page-actions" class="page-actions-menu__list">
{{#toolbar}}
<li id="{{name}}" class="page-actions-menu__list-item">
{{#components}}
<a id="{{id}}" href="{{href}}" class="{{class}}" data-event-name="{{data-event-name}}" role="button" title="{{title}}">
{{text}}
</a>
{{/components}}
</li>
{{/toolbar}}
{{#overflowMenu}}
<li id="{{item-id}}" class="page-actions-menu__list-item">
<input type="checkbox" id="toolbar-overflow-menu__checkbox" role="button" aria-label="{{text}}" aria-expanded="false" >
<label class="toolbar-overflow-menu__button {{class}}" title="{{title}}" for="toolbar-overflow-menu__checkbox">
{{text}}
</label>
<ul class="toolbar-overflow-menu__list">
{{#pageActions}}
<li id="{{name}}">
{{#components}}
<a id="{{id}}" href="{{href}}" class="toolbar-overflow-menu__list-item {{class}}" title="{{title}}">
{{text}}
</a>
{{/components}}
</li>
{{/pageActions}}
</ul>
</li>
{{/overflowMenu}}
</ul>
</nav>

View File

@ -87,96 +87,13 @@
flex-grow: 0;
}
#toolbar-overflow-menu__checkbox {
// Always occlude the checkbox. The checkbox display cannot be none since its focus state is used
// for other selectors.
position: absolute;
z-index: @z-indexOccluded;
opacity: 0;
}
.toolbar-overflow-menu__button {
// Use the hand icon for the overflow button which is actually a checkbox label.
cursor: pointer;
}
#toolbar-overflow-menu__checkbox:focus + .toolbar-overflow-menu__button {
// The overflow button / label itself cannot receive focus but the underlying checkbox can. Keep
// the button and checkbox focus presentation in sync. From
// resources/src/mediawiki.toc.styles/screen.less.
outline: dotted 1px; /* Firefox style for focus */
outline: auto @colorProgressiveHighlight; /* Webkit style for focus */
}
.touch-events #toolbar-overflow-menu__checkbox:focus + .toolbar-overflow-menu__button {
// Buttons have no focus outline on mobile.
outline: 0;
}
.toolbar-overflow-menu__list {
.page-actions-overflow-list {
// The top of the menu is flush with the bottom of the page actions toolbar.
position: absolute;
top: 100%;
right: 0;
//
// A variable max-height is set in JavaScript so a minimum height is needed.
min-height: 200px;
//
// If the height exceeds the maximum allowed, add a vertical scrollbar.
overflow-y: auto;
//
// The menu floats over content but below overlays.
z-index: @z-indexDrawer;
//
font-size: @pageActionFontSize;
font-weight: bold;
background: @skinContentBgColor;
box-shadow: 0 5px 17px 0 rgba( 0, 0, 0, 0.24 ), 0 0 1px @colorGray10;
border-radius: @borderRadius;
//
visibility: hidden;
opacity: 0;
transform: translateY( -8px );
// Animate menu visibility, opacity, and translation changes in and out. Visibility must be
// animated since it's a boolean and nothing can be seen in display hidden. Visibility itself
// cannot be animated as it causes a flash on page load in Chromium due to
// https://bugs.chromium.org/p/chromium/issues/detail?id=332189. The effect is that the menu is
// animated in but not animated out.
.transition( opacity .1s ease-in-out, transform .1s ease-in-out; );
}
.toolbar-overflow-menu__list-item {
// Fill the list item cell.
.box-sizing( border-box );
display: inline-block;
width: 100%;
//
padding: 1em;
white-space: nowrap;
// Left-align text. Button elements are centered.
text-align: left;
//
color: @grayMediumDark;
&:visited, &:active {
// Visited and active links need extra specificity.
color: @grayMediumDark;
}
//
// Make the app feel like an app, not a JPEG. When hovering over a menu item, add a little
// interactivity.
&:hover {
text-decoration: none;
background: @grayLightest;
}
}
#toolbar-overflow-menu__checkbox:checked ~ .toolbar-overflow-menu__list {
// Reveal the overflow menu when checked.
visibility: visible;
opacity: 1;
transform: translateY( 0 );
}
// overriding common.less `display:inherit` (which causes `display: flex;` in this instance).

View File

@ -1,6 +1,7 @@
( function ( M ) {
var
mobile = M.require( 'mobile.startup' ),
ToggleList = M.require( 'skins.minerva.scripts/ToggleList' ),
downloadPageAction = M.require( 'skins.minerva.scripts/downloadPageAction' ),
Icon = mobile.Icon,
skin = M.require( 'mobile.init/skin' ),
@ -8,11 +9,7 @@
toolbarSelector = '.page-actions-menu',
/** The secondary overflow submenu component container. */
overflowSubmenuSelector = '#page-actions-overflow',
/** The visible label icon associated with the checkbox. */
overflowButtonSelector = '.toolbar-overflow-menu__button',
/** The underlying hidden checkbox that controls secondary overflow submenu visibility. */
overflowCheckboxSelector = '#toolbar-overflow-menu__checkbox',
overflowListSelector = '.toolbar-overflow-menu__list';
overflowListSelector = '.toggle-list__list';
/**
* @param {Window} window
@ -21,16 +18,9 @@
* @return {void}
*/
function bind( window, toolbar, eventBus ) {
var
overflowSubmenu = toolbar.querySelector( overflowSubmenuSelector ),
overflowButton = toolbar.querySelector( overflowButtonSelector ),
overflowCheckbox = toolbar.querySelector( overflowCheckboxSelector ),
overflowList = toolbar.querySelector( overflowListSelector );
var overflowSubmenu = toolbar.querySelector( overflowSubmenuSelector );
if ( overflowSubmenu ) {
bindOverflowSubmenu(
window, overflowSubmenu, overflowButton, overflowCheckbox, overflowList, eventBus
);
ToggleList.bind( window, overflowSubmenu, eventBus, true );
}
}
@ -40,65 +30,13 @@
* @return {void}
*/
function render( window, toolbar ) {
var overflowList = toolbar.querySelector( overflowListSelector );
var
overflowSubmenu = toolbar.querySelector( overflowSubmenuSelector ),
overflowList = toolbar.querySelector( overflowListSelector );
renderEditButton();
renderDownloadButton( window, overflowList );
if ( overflowList ) {
resizeOverflowList( overflowList );
}
}
/**
* Automatically dismiss the submenu when clicking or focusing elsewhere, resize the menu on
* scroll and window resize, and update the aria-expanded attribute based on submenu visibility.
* @param {Window} window
* @param {Element} submenu
* @param {Element} button
* @param {HTMLInputElement} checkbox
* @param {Element} list
* @param {OO.EventEmitter} eventBus
* @return {void}
*/
function bindOverflowSubmenu( window, submenu, button, checkbox, list, eventBus ) {
var
resize = resizeOverflowList.bind( undefined, list ),
updateAriaExpanded = function () {
checkbox.setAttribute( 'aria-expanded', ( !!checkbox.checked ).toString() );
};
window.addEventListener( 'click', function ( event ) {
if ( event.target !== button && event.target !== checkbox ) {
// Something besides the button or checkbox was tapped. Dismiss the submenu.
checkbox.checked = false;
updateAriaExpanded();
}
} );
// If focus is given to any element outside the menu, dismiss the submenu. Setting a
// focusout listener on submenu would be preferable, but this interferes with the click
// listener.
window.addEventListener( 'focusin', function ( event ) {
if ( event.target instanceof Node && !submenu.contains( event.target ) ) {
// Something besides the button or checkbox was focused. Dismiss the menu.
checkbox.checked = false;
updateAriaExpanded();
}
} );
eventBus.on( 'scroll:throttled', resize );
eventBus.on( 'resize:throttled', resize );
checkbox.addEventListener( 'change', updateAriaExpanded );
}
/**
* @param {HTMLElement} list
* @return {void}
*/
function resizeOverflowList( list ) {
var rect = list.getClientRects()[ 0 ];
if ( rect ) {
list.style.maxHeight = window.document.documentElement.clientHeight - rect.top + 'px';
if ( overflowSubmenu ) {
ToggleList.render( overflowSubmenu, true );
}
}

View File

@ -125,7 +125,7 @@
*/
function downloadPageAction( skin, supportedNamespaces, windowObj, hasText ) {
var
modifier = hasText ? 'toolbar-overflow-menu__list-item' : 'mw-ui-icon-element',
modifier = hasText ? 'toggle-list-item__anchor--menu' : 'mw-ui-icon-element',
icon,
spinner = icons.spinner( {
hasText: hasText,

View File

@ -198,6 +198,9 @@
"styles": [
"resources/skins.minerva.base.styles/reset.less",
"resources/skins.minerva.base.styles/ui.less",
"components/ToggleList/ToggleList.less",
"components/ToggleList/DropDownList.less",
"components/ToggleList/MenuListItem.less",
"resources/skins.minerva.base.styles/pageactions.less",
"resources/skins.minerva.base.styles/common.less",
"resources/skins.minerva.base.styles/images.less",
@ -511,6 +514,7 @@
"resources/skins.minerva.scripts/pageIssues.js",
"resources/skins.minerva.scripts/UriUtil.js",
"resources/skins.minerva.scripts/TitleUtil.js",
"components/ToggleList/ToggleList.js",
"resources/skins.minerva.scripts/Toolbar.js",
"resources/skins.minerva.scripts/init.js",
"resources/skins.minerva.scripts/initLogging.js",