Remove the mw-ui-icon hacks and overrides

Note: this agitates T230232 again - when merging this please
make sure a merge for I929090848f3e04647a97f4979ec78682623fa070
is pending.

In various places we try to override the default mw-ui-icon behaviours
The hacks need to be removed as part of addressing the core problem.

Changes:
* Wherever we use mw-ui-icon-before in PHP - wrap the label with a span
so that label font-size is altered where needed - not the icon
* Where a small icon is needed us isSmall parameter for the Icon component
* Apply font-size to labels of mw-ui-icon-before elements
* The browser tests need a slight update to access the span element inside
a menu item - in the case of the logout button the label is always hidden,
so we need to check the visibility of the parent element (secondary_action)

Bug: T229440
Depends-On:  I3f803ec4c9068b30aa93b803391aa4d65d8310ff
Change-Id: I07e4ae233979636b739f1117dd7703571e0a9366
This commit is contained in:
jdlrobson 2019-08-30 10:23:55 -07:00
parent 6cd0d4b097
commit c0f08790ea
30 changed files with 94 additions and 133 deletions

View file

@ -1 +1,2 @@
{{> ToggleList/ToggleList}}
{{> ToggleList/ToggleList}}
<!-- version 1.0.1 - used for partial template invalidation -->

View file

@ -3,30 +3,39 @@
@import '../../minerva.less/minerva.variables.less';
@import '../../minerva.less/minerva.mixins.less';
.toggle-list-item__anchor--menu {
font-size: @font-size-minerva-small;
font-weight: bold;
// Fill the list item cell.
.box-sizing( border-box );
.toggle-list-item {
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;
}
}
.toggle-list-item__anchor {
display: block;
&:hover {
text-decoration: none;
}
&:visited, &:active {
color: @grayMediumDark;
}
}
.toggle-list-item__icon {
vertical-align: middle;
}
.toggle-list-item__label {
// Left-align text. Button elements are centered.
text-align: left;
color: @grayMediumDark;
font-weight: bold;
white-space: nowrap;
vertical-align: middle;
font-size: @font-size-minerva-small;
}

View file

@ -9,6 +9,8 @@
}
.toggle-list__toggle {
// labels are inline by default and this is also an icon
display: inline-block;
// Use the hand icon for the toggle button which is actually a checkbox label.
cursor: pointer;
}

View file

@ -24,7 +24,7 @@
data-event-name="{{analyticsEventName}}">
{{text}}
</label>
<ul class="toggle-list__list {{listClass}}">
<ul class="toggle-list__list new {{listClass}}">
{{#items}}
{{> ToggleList/ToggleListItem}}
{{/items}}

View file

@ -7,6 +7,9 @@
}}
{{#components}}
<li class="toggle-list-item">
<a class="toggle-list-item__anchor {{class}}" href="{{href}}" data-event-name="{{data-event-name}}">{{text}}</a>
<a class="toggle-list-item__anchor" href="{{href}}" data-event-name="{{data-event-name}}">
<span class="toggle-list-item__icon {{class}}"></span>
<span class="toggle-list-item__label">{{text}}</span>
</a>
</li>
{{/components}}

View file

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

View file

@ -218,7 +218,7 @@ class ToolbarBuilder {
$msg = $this->messageLocalizer->msg( 'watchthispage' );
$icon = 'watch';
}
$iconClass = MinervaUI::iconClass( $icon, 'element', 'watch-this-article' ) . ' jsonly';
$iconClass = MinervaUI::iconClass( $icon, 'element', 'watch-this-article', 'mf' ) . ' jsonly';
if ( $isWatched ) {
$iconClass .= ' watched';
}

View file

@ -94,7 +94,7 @@ class UserNamespaceOverflowBuilder implements IOverflowBuilder {
$this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->title ),
$this->messageLocalizer,
MinervaUI::iconClass( 'language-switcher-base20', 'before',
'minerva-page-actions-language-switcher toggle-list-item__anchor--menu' ),
'minerva-page-actions-language-switcher' ),
'minerva-page-actions-language-switcher'
) );
}
@ -144,7 +144,7 @@ class UserNamespaceOverflowBuilder implements IOverflowBuilder {
'page-actions-overflow-' . $name,
$href,
MinervaUI::iconClass( '', 'before',
'wikimedia-ui-' . $icon . '-base20 toggle-list-item__anchor--menu'
'wikimedia-ui-' . $icon . '-base20'
),
$this->messageLocalizer->msg( 'minerva-page-actions-' . $name ),
$name

View file

@ -54,12 +54,6 @@ final class UserMenuDirector {
public function renderMenuData( array $personalTools ) {
$entries = $this->builder->getGroup( $personalTools )->getEntries();
foreach ( $entries as &$entry ) {
foreach ( $entry['components'] as &$component ) {
$component['class'] .= ' toggle-list-item__anchor--menu';
}
}
$templateParser = new TemplateParser( __DIR__ . '/../../../components' );
return empty( $entries )
? null

View file

@ -122,7 +122,7 @@ class MinervaTemplate extends BaseTemplate {
$action = Action::getActionName( RequestContext::getMain() );
if ( isset( $data['historyLink'] ) && $action === 'view' ) {
$args = [
'clockIconClass' => MinervaUI::iconClass( 'clock', 'before' ),
'clockIconClass' => MinervaUI::iconClass( 'clock', 'before', 'mw-ui-icon-small' ),
'arrowIconClass' => MinervaUI::iconClass(
'expand-gray', 'element',
// FIXME: `mw-ui-icon-mf-arrow-gray` can be removed from list of classes

View file

@ -1,11 +1,10 @@
<div class="last-modified-bar view-border-box footer-element">
<div class="content">
<div class="last-modifier-tagline truncated-text {{clockIconClass}}">
<div class="{{arrowIconClass}}"></div><a href="{{href}}"
<span class="{{arrowIconClass}}"></span><a href="{{href}}"
data-user-name="{{data-user-name}}"
data-user-gender="{{data-user-gender}}"
data-timestamp="{{data-timestamp}}">
{{text}}
data-timestamp="{{data-timestamp}}"><span>{{text}}</span>
</a>
</div>
</div>

View file

@ -63,4 +63,4 @@
</div>
</div>
<div class="mw-notification-area" data-mw="interface"></div>
<!-- v:8.1.9 -->
<!-- v:8.1.10 -->

View file

@ -19,8 +19,6 @@
@font-size-minerva-small: unit( 14 / @font-size-browser, em ); // Equals `14px` at `16px` browser default.
@tocFontSize: @font-size-minerva-small;
@indicatorFontSize: 0.4em;
// colors
@grayMediumLight: @colorGray10;
@grayMediumDark: @colorGray5;
@ -64,7 +62,6 @@
// Page actions
@taglineFontSize: 0.85em;
@pageActionBorder: 1px;
@pageActionToolbarHeight: 44px; // total height is 46px. 2px added by border on .page-actions-menu
// colors
@chromeColor: @grayLightest;

View file

@ -1,4 +1,6 @@
@import 'mediawiki.ui/variables';
.minerva--history-page-action-enabled {
.page-actions-menu__list-item {
flex-basis: auto;

View file

@ -22,15 +22,22 @@ footer {
background-color: @lastModifiedBarBgColor;
display: block;
color: @lastModifiedBarTextColor;
line-height: 1.5em;
transition: background-color 0.2s ease, color 0.2s ease;
}
.last-modifier-tagline {
@end-padding: @iconGutterWidth + @iconGutterWidth + @iconSize;
display: block;
width: 100%;
font-size: @font-size-minerva-small;
padding: 7px 2em 7px 0;
padding: 7px @end-padding 7px 0;
.truncated-text();
> a,
> span > span,
> span:last-child {
display: inline-block;
font-size: @font-size-minerva-small;
}
}
.indicator {

View file

@ -35,43 +35,18 @@
.page-actions-menu__list {
display: flex;
height: @iconSize * 2;
justify-content: space-between;
height: @pageActionToolbarHeight;
}
.page-actions-menu__list-item {
display: flex;
flex-basis: 4em;
justify-content: flex-end;
justify-content: flex-start;
align-items: center;
// overriding default icon styles
.mw-ui-icon-element {
// The page actions menu icons are ever so slightly larger
// than standard icons.
@pageActionsIconSize: @iconSize + 0.15;
@pageActionsIconTarget: 2.75em; // 44px minimum touch target
// explicitly added to ensure this element (which is an anchor) receives width/height
// when it's viewed in services that manipulate DOM such as Google Translate.
display: block;
position: relative;
min-width: @pageActionsIconTarget;
width: @pageActionsIconTarget;
height: @pageActionsIconTarget;
&:hover {
box-shadow: none;
}
&:before {
margin: 0;
// `.mw-ui-icon` absolutely positions this pseudo-element but only
// positions right & left. This ensures icon stretches 100% height and
// stretches the entire height of its parent element.
top: 0;
bottom: 0;
background-size: @pageActionsIconSize;
}
li > *:hover {
box-shadow: none;
}
}

View file

@ -184,30 +184,14 @@ input.search {
}
.content {
// Correct icon sizes of edit icon when using mw-ui-icon.
h1 {
.edit-page {
font-size: 1 / @fontSizeH1;
}
}
h2 {
// Clear table of contents and any other floated elements in desktop Minerva.
clear: left;
.edit-page {
font-size: 1 / @fontSizeH2;
}
}
h3 {
.edit-page {
font-size: 1 / @fontSizeH3;
}
}
.edit-page {
display: block;
font-size: initial;
}
.collapsible-heading .edit-page {
@ -227,7 +211,7 @@ input.search {
margin-bottom: @headingMargin;
.indicator {
font-size: @indicatorFontSize;
font-size: initial;
margin-left: -@iconGutterWidth;
}
}

View file

@ -7,7 +7,7 @@
// Fill empty space left / start so the buttons can stay right / end.
width: 100%;
// Keep space for at least two buttons.
min-width: 2 * 43px;
min-width: 2 * @menuButtonIconSize;
// Center vertically.
align-items: center;
// Layout from right / end.
@ -17,16 +17,10 @@
.mw-ui-icon-element {
display: block;
width: 43px;
min-width: 43px;
&:before {
margin: 0 11.5px;
}
}
.toggle-list {
width: 43px;
width: @menuButtonIconSize;
display: inline-block;
}
}

View file

@ -26,7 +26,6 @@ table.ambox {
.client-js .ambox {
cursor: pointer;
font-size: 0.8em;
width: auto;
background: @amboxBackground;
color: @colorGray5;
@ -61,6 +60,11 @@ table.ambox {
td {
position: relative;
padding: @amboxPadding @amboxPadding @amboxPadding @amboxIconPadding;
> div,
> span {
font-size: 0.8em;
}
}
// All text should be treated the same
@ -101,10 +105,7 @@ table.ambox {
.mw-ui-icon {
position: absolute;
left: -@amboxPadding;
}
.mw-ui-icon:before {
background-size: 75%;
top: @amboxPadding * 2;
}
.ambox-learn-more {

View file

@ -36,7 +36,6 @@
border-top: 1px solid @colorGray14;
// offset the border for the icon by 1px
margin-top: -1px;
font-size: 0.875em;
&:first-child {
border-top: 0;
@ -45,12 +44,12 @@
a {
color: @colorGray5;
display: block;
padding: @menuLinkLineHeight / 2 10px @menuLinkLineHeight / 2 15px;
// Overflowing text is ellipsized.
max-width: 100%;
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
padding: @menuLinkLineHeight / 2 10px @menuLinkLineHeight / 2 15px;
&:hover {
box-shadow: inset 4px 0 0 0 @colorProgressive;
@ -62,13 +61,9 @@
color: @colorGray5;
}
&.mw-ui-icon {
span {
font-size: @font-size-minerva-small;
font-weight: bold;
line-height: 1.857; // equals `26px` at `font-size: 14px` above
&:before {
font-size: 16px;
}
}
}
}

View file

@ -126,7 +126,7 @@
*/
function downloadPageAction( page, supportedNamespaces, windowObj, hasText ) {
var
modifier = hasText ? 'toggle-list-item__anchor--menu' : 'mw-ui-icon-element',
modifier = hasText ? 'toggle-list-item__anchor toggle-list-item__label' : 'mw-ui-icon-element',
icon,
spinner = icons.spinner( {
hasText: hasText,
@ -149,11 +149,11 @@
click: getOnClickHandler( spinner )
},
hasText: hasText,
label: hasText ? mw.msg( 'minerva-download' ) : '',
label: mw.msg( 'minerva-download' ),
modifier: modifier
} );
return $( '<li>' ).addClass( 'page-actions-menu__list-item' ).append( icon.$el ).append( spinner.$el.hide() );
return $( '<li>' ).addClass( hasText ? 'toggle-list-item' : 'page-actions-menu__list-item' ).append( icon.$el ).append( spinner.$el.hide() );
} else {
return null;
}

View file

@ -154,7 +154,9 @@
$bar.find( '.mw-ui-icon-minerva-clock' ).addClass( 'mw-ui-icon-minerva-clock-invert' );
$bar.find( '.mw-ui-icon-mf-expand-gray' ).addClass( 'mw-ui-icon-mf-expand-invert' );
}
msg = time.getLastModifiedMessage( ts, username, gender, historyUrl );
msg = $( '<span>' ).html(
time.getLastModifiedMessage( ts, username, gender, historyUrl )
);
$lastModifiedLink.replaceWith( msg );
}
}

View file

@ -1,5 +1,5 @@
<li class="{{class}}">
{{#components}}
<a href="{{href}}" class="{{class}}" data-event-name="{{data-event-name}}">{{text}}</a>
<a href="{{href}}" class="{{class}}" data-event-name="{{data-event-name}}"><span>{{text}}</span></a>
{{/components}}
</li>

View file

@ -20,7 +20,9 @@
issue.$el.find( '.mbox-text' ),
$clickContainer = multiple ? issue.$el.parents( '.mbox-text' ) : issue.$el;
$issueContainer.prepend( issue.iconString );
$issueContainer.prepend(
issue.issue.icon.$el.clone().addClass( 'mw-ui-icon-small' )
);
$issueContainer.prepend( $learnMoreEl );
$clickContainer.on( 'click', function () {

View file

@ -208,8 +208,6 @@
return {
issue: pageIssue,
$el: $box,
// For template compatibility with PageIssuesOverlay
iconString: pageIssue.icon.toHtmlString(),
text: $container.html()
};
}

View file

@ -17,11 +17,4 @@
}
}
}
.mw-ui-icon-talk {
&:before {
// FIXME: this shouldn't be necessary
margin-right: 0.5em;
}
}
}

View file

@ -11,9 +11,7 @@
.clear {
position: absolute;
// the icon should take into account overlay-title top padding
// and then be centered
top: ( @iconSize / 2 ) + @headerVerticalPadding;
top: 1em;
right: 0;
}
}

View file

@ -27,7 +27,6 @@
grouped: true,
icon: icon
},
iconString: this.sandbox.match.typeOf( 'string' ),
text: '<p>Smelly</p>'
},
'When the box is a child of mw-collapsible-content it grouped'
@ -42,7 +41,6 @@
grouped: false,
icon: icon
},
iconString: this.sandbox.match.typeOf( 'string' ),
text: '<p>Dirty</p>'
},
'When the box is not child of mw-collapsible-content it !grouped'

View file

@ -10,11 +10,15 @@ const iClickOnTheMainNavigationButton = () => {
};
const iShouldSeeAUserPageLinkInMenu = () => {
ArticlePage.menu_element.element( '.mw-ui-icon-minerva-profile' );
ArticlePage.menu_element.element( '.primary-action' );
};
const iShouldSeeLogoutLinkInMenu = () => {
ArticlePage.menu_element.element( '.secondary-action' );
};
const iShouldSeeALinkInMenu = ( text ) => {
assert.strictEqual( ArticlePage.menu_element.element( `=${text}` ).isVisible(),
assert.strictEqual( ArticlePage.menu_element.element( `span=${text}` ).isVisible(),
true, `Link to ${text} is visible.` );
};
@ -26,5 +30,6 @@ const iShouldSeeALinkToDisclaimer = () => {
module.exports = {
iClickOnTheMainNavigationButton,
iSeeALinkToAboutPage, iShouldSeeAUserPageLinkInMenu,
iShouldSeeLogoutLinkInMenu,
iShouldSeeALinkInMenu, iShouldSeeALinkToDisclaimer
};

View file

@ -3,6 +3,7 @@ const {
iAmOnPage
} = require( '../features/step_definitions/common_steps' ),
{ iSeeALinkToAboutPage, iShouldSeeAUserPageLinkInMenu,
iShouldSeeLogoutLinkInMenu,
iClickOnTheMainNavigationButton,
iShouldSeeALinkInMenu, iShouldSeeALinkToDisclaimer
} = require( '../features/step_definitions/menu_steps' );
@ -20,10 +21,11 @@ describe( 'Menus open correct page for anonymous users', () => {
iShouldSeeALinkToDisclaimer();
iShouldSeeAUserPageLinkInMenu();
iSeeALinkToAboutPage();
[ 'Log out', 'Home', 'Random', 'Settings', 'Contributions',
[ 'Home', 'Random', 'Settings', 'Contributions',
'Watchlist' ].forEach( ( label ) => {
iShouldSeeALinkInMenu( label );
} );
iShouldSeeLogoutLinkInMenu();
try {
iShouldSeeALinkInMenu( 'Nearby' );
} catch ( e ) {