Bartosz Dziewoński has uploaded a new change for review.
https://gerrit.wikimedia.org/r/179786
Change subject: [BREAKING CHANGE] PopupToolGroup and friends: Pay off technical
debt
......................................................................
[BREAKING CHANGE] PopupToolGroup and friends: Pay off technical debt
PopupToolGroup.less
Reimplemented the structure as a single table with three cells,
rather than inline-block icon and inline-table with two cells.
This fixes rendering glitches on IE11 (T74592, T74640) and
incompatibility with ClippableElement (T78447).
Tool.js
Changed DOM structure to support PopupToolGroup changes.
Before: After:
<a> <a>
<span>Icon</span> <span>Icon</span>
<span> <span>Title</span>
<span>Title</span> <span>Accel</span>
<span>Accel</span> </a>
</span>
</a>
ListToolGroup.less
ListToolGroup.js
Switched from display: inline-block to block, all it ever did was
cause me headache.
Elsewhere
Tweaks to match changes in Tool and PopupToolGroup.
Bug: T78499
Change-Id: Idc299e235b65317157e1022e3bebcc8cee8a2603
---
M src/Tool.js
M src/styles/toolgroups/BarToolGroup.less
M src/styles/toolgroups/ListToolGroup.less
M src/styles/toolgroups/MenuToolGroup.less
M src/styles/toolgroups/PopupToolGroup.less
M src/themes/apex/tools.less
M src/themes/mediawiki/tools.less
M src/toolgroups/ListToolGroup.js
8 files changed, 64 insertions(+), 78 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/86/179786/1
diff --git a/src/Tool.js b/src/Tool.js
index ed2d02e..279a4cd 100644
--- a/src/Tool.js
+++ b/src/Tool.js
@@ -28,7 +28,6 @@
this.toolbar = this.toolGroup.getToolbar();
this.active = false;
this.$title = this.$( '<span>' );
- this.$titleText = this.$( '<span>' );
this.$accel = this.$( '<span>' );
this.$link = this.$( '<a>' );
this.title = null;
@@ -37,7 +36,7 @@
this.toolbar.connect( this, { updateState: 'onUpdateState' } );
// Initialization
- this.$titleText.addClass( 'oo-ui-tool-title-text' );
+ this.$title.addClass( 'oo-ui-tool-title' );
this.$accel
.addClass( 'oo-ui-tool-accel' )
.prop( {
@@ -46,12 +45,9 @@
dir: 'ltr',
lang: 'en'
} );
- this.$title
- .addClass( 'oo-ui-tool-title' )
- .append( this.$titleText, this.$accel );
this.$link
.addClass( 'oo-ui-tool-link' )
- .append( this.$icon, this.$title )
+ .append( this.$icon, this.$title, this.$accel )
.prop( 'tabIndex', 0 )
.attr( 'role', 'button' );
this.$element
@@ -239,7 +235,7 @@
accel = this.toolbar.getToolAccelerator(
this.constructor.static.name ),
tooltipParts = [];
- this.$titleText.text( this.title );
+ this.$title.text( this.title );
this.$accel.text( accel );
if ( titleTooltips && typeof this.title === 'string' &&
this.title.length ) {
diff --git a/src/styles/toolgroups/BarToolGroup.less
b/src/styles/toolgroups/BarToolGroup.less
index 055b8a4..a0d51bb 100644
--- a/src/styles/toolgroups/BarToolGroup.less
+++ b/src/styles/toolgroups/BarToolGroup.less
@@ -18,6 +18,7 @@
display: block;
}
+ .oo-ui-tool-accel,
.oo-ui-tool-title {
display: none;
}
diff --git a/src/styles/toolgroups/ListToolGroup.less
b/src/styles/toolgroups/ListToolGroup.less
index ff46a7f..f3c745a 100644
--- a/src/styles/toolgroups/ListToolGroup.less
+++ b/src/styles/toolgroups/ListToolGroup.less
@@ -2,15 +2,12 @@
.oo-ui-listToolGroup {
.oo-ui-tool {
- display: inline-block;
- width: 100%;
+ display: block;
.oo-ui-box-sizing(border-box);
&-link {
- display: block;
cursor: pointer;
- white-space: nowrap;
}
&.oo-ui-widget-disabled {
diff --git a/src/styles/toolgroups/MenuToolGroup.less
b/src/styles/toolgroups/MenuToolGroup.less
index f7e836c..cfbbe0b 100644
--- a/src/styles/toolgroups/MenuToolGroup.less
+++ b/src/styles/toolgroups/MenuToolGroup.less
@@ -5,9 +5,7 @@
display: block;
&-link {
- display: block;
cursor: pointer;
- white-space: nowrap;
}
&.oo-ui-widget-disabled {
diff --git a/src/styles/toolgroups/PopupToolGroup.less
b/src/styles/toolgroups/PopupToolGroup.less
index 5dd4990..1b54bea 100644
--- a/src/styles/toolgroups/PopupToolGroup.less
+++ b/src/styles/toolgroups/PopupToolGroup.less
@@ -47,38 +47,25 @@
}
.oo-ui-tool-link {
- .oo-ui-iconElement-icon {
- display: inline-block;
+ display: table;
+ width: 100%;
+ vertical-align: middle;
+ white-space: nowrap;
+
+ .oo-ui-iconElement-icon,
+ .oo-ui-tool-accel,
+ .oo-ui-tool-title {
+ display: table-cell;
vertical-align: middle;
}
- .oo-ui-tool-title {
- display: inline-table;
- vertical-align: middle;
- width: 100%;
- // HACK: We take 100% of available width, but there's
already an icon taking space before this element.
- // Offset by 2.25em to match icon width+margin, 0.25em
more to avoid border overlap
- margin-right: -2.5em;
+ .oo-ui-tool-accel {
+ text-align: right;
+ }
- .oo-ui-tool-title-text,
- .oo-ui-tool-accel {
- display: table-cell;
- }
-
- .oo-ui-tool-title-text {
- width: 100%;
- }
-
- .oo-ui-tool-accel {
- text-align: right;
- // HACK: 2.25em to match icon width+margin,
0.25em more to avoid border overlap
- padding-right: 2.5em;
- }
-
- .oo-ui-tool-accel:not(:empty) {
- // Push away from tool's title
- padding-left: 3em;
- }
+ .oo-ui-tool-accel:not(:empty) {
+ // Push away from tool's title
+ padding-left: 3em;
}
}
diff --git a/src/themes/apex/tools.less b/src/themes/apex/tools.less
index 5e48929..c16d054 100644
--- a/src/themes/apex/tools.less
+++ b/src/themes/apex/tools.less
@@ -187,15 +187,21 @@
.oo-ui-iconElement-icon {
height: 2em;
width: 2em;
- margin-right: 0.25em;
+ min-width: 2em;
}
.oo-ui-tool-title {
+ padding-left: 0.25em;
+ }
+
+ .oo-ui-tool-accel,
+ .oo-ui-tool-title {
line-height: 2em;
font-size: 0.8em;
- .oo-ui-tool-accel {
- color: #888;
- }
+ }
+
+ .oo-ui-tool-accel {
+ color: #888;
}
}
}
@@ -212,10 +218,7 @@
.oo-ui-tool {
border: solid 1px transparent;
margin: -1px 0;
-
- &-link {
- padding-right: 0.5em;
- }
+ padding: 0 0.25em 0 0;
&-active {
&.oo-ui-widget-enabled {
@@ -245,15 +248,18 @@
}
&.oo-ui-widget-disabled {
- .oo-ui-tool-link .oo-ui-tool-title {
- color: #ccc;
+ .oo-ui-tool-link {
+ .oo-ui-tool-title {
+ color: #ccc;
+ }
+
.oo-ui-tool-accel {
color: #ddd;
}
- }
- .oo-ui-tool-link .oo-ui-iconElement-icon {
- opacity: 0.2;
+ .oo-ui-iconElement-icon {
+ opacity: 0.2;
+ }
}
}
}
@@ -289,12 +295,9 @@
}
.oo-ui-tool {
- &-link {
- padding: 0 1em 0 0.25em;
- display: block;
- cursor: pointer;
- white-space: nowrap;
+ padding: 0 0.75em 0 0.25em;
+ &-link {
.oo-ui-iconElement-icon {
background-image: none;
}
diff --git a/src/themes/mediawiki/tools.less b/src/themes/mediawiki/tools.less
index be95501..6f76a5d 100644
--- a/src/themes/mediawiki/tools.less
+++ b/src/themes/mediawiki/tools.less
@@ -30,8 +30,11 @@
margin: 0.3em;
&.oo-ui-widget-enabled {
- .oo-ui-tool-link .oo-ui-tool-title {
- color: #000;
+ .oo-ui-tool-link {
+ .oo-ui-tool-title {
+ color: #000;
+ }
+
.oo-ui-tool-accel {
color: #888;
}
@@ -144,9 +147,14 @@
.oo-ui-iconElement-icon {
height: 2em;
width: 2em;
- margin-right: 0.25em;
+ min-width: 2em;
}
+ .oo-ui-tool-title {
+ padding-left: 0.25em;
+ }
+
+ .oo-ui-tool-accel,
.oo-ui-tool-title {
line-height: 2em;
font-size: 0.8em;
@@ -156,11 +164,7 @@
.theme-oo-ui-listToolGroup () {
.oo-ui-tool {
- padding: 0 0.25em;
-
- &-link {
- padding-right: 0.5em;
- }
+ padding: 0 0.5em 0 0.25em;
&.oo-ui-widget-enabled {
&:hover {
@@ -176,15 +180,18 @@
}
&.oo-ui-widget-disabled {
- .oo-ui-tool-link .oo-ui-tool-title {
- color: #ccc;
+ .oo-ui-tool-link {
+ .oo-ui-tool-title {
+ color: #ccc;
+ }
+
.oo-ui-tool-accel {
color: #ddd;
}
- }
- .oo-ui-tool-link .oo-ui-iconElement-icon {
- opacity: 0.2;
+ .oo-ui-iconElement-icon {
+ opacity: 0.2;
+ }
}
}
}
@@ -231,12 +238,9 @@
}
.oo-ui-tool {
- &-link {
- padding: 0 1em 0 0.25em;
- display: block;
- cursor: pointer;
- white-space: nowrap;
+ padding: 0 0.75em 0 0.25em;
+ &-link {
.oo-ui-iconElement-icon {
background-image: none;
}
diff --git a/src/toolgroups/ListToolGroup.js b/src/toolgroups/ListToolGroup.js
index 357e97b..6865746 100644
--- a/src/toolgroups/ListToolGroup.js
+++ b/src/toolgroups/ListToolGroup.js
@@ -77,7 +77,7 @@
// 'display' attribute and restores it, and the tool uses a <span> and
can be hidden and re-shown.
// Is this a jQuery bug? http://jsfiddle.net/gtj4hu3h/
if ( this.getExpandCollapseTool().$element.css( 'display' ) ===
'inline' ) {
- this.getExpandCollapseTool().$element.css( 'display',
'inline-block' );
+ this.getExpandCollapseTool().$element.css( 'display', 'block' );
}
this.updateCollapsibleState();
--
To view, visit https://gerrit.wikimedia.org/r/179786
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc299e235b65317157e1022e3bebcc8cee8a2603
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
