jenkins-bot has submitted this change and it was merged.
Change subject: Theme: Add theme classes to $icon and $indicator only
......................................................................
Theme: Add theme classes to $icon and $indicator only
Previously we added them to $element, which meant that they continued
to apply to nested widgets, which proved problematic in
PopupButtonWidget.
Also remove now-unnecessary CSS selectors.
This would be a breaking change for third-party themes, if any existed.
We need to call this.updateThemeClasses() in some more places, for the
benefit of tests (where updateThemeClasses is synchronous). If it is
not called after this.$icon (from IconElement) and this.$indicator are
set up, the theme classes on them will not be updated.
Bug: T109625
Change-Id: Ie93e4d6ed5637cdd95367729e1349a6da923f461
---
M php/Element.php
M php/Theme.php
M src/Theme.js
M src/mixins/IconElement.js
M src/mixins/IndicatorElement.js
M src/themes/mediawiki/icons.json
M src/themes/mediawiki/indicators.json
7 files changed, 43 insertions(+), 8 deletions(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/php/Element.php b/php/Element.php
index 601f2b8..a7bd683 100644
--- a/php/Element.php
+++ b/php/Element.php
@@ -130,7 +130,7 @@
* @return Tag|null Target property or null if not found
*/
public function __get( $name ) {
- // Search mixins for methods
+ // Search mixins for the property
foreach ( $this->mixins as $mixin ) {
if ( isset( $mixin::$targetPropertyName ) &&
$mixin::$targetPropertyName === $name ) {
return $mixin->target;
@@ -142,6 +142,22 @@
}
/**
+ * Check for existence of a mixed-in target property.
+ *
+ * @param string $name Property name
+ * @return bool Whether property exists
+ */
+ public function __isset( $name ) {
+ // Search mixins for the property
+ foreach ( $this->mixins as $mixin ) {
+ if ( isset( $mixin::$targetPropertyName ) &&
$mixin::$targetPropertyName === $name ) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
* Get the HTML tag name.
*
* Override this method to base the result on instance information.
diff --git a/php/Theme.php b/php/Theme.php
index 38f6149..7f8eea3 100644
--- a/php/Theme.php
+++ b/php/Theme.php
@@ -58,8 +58,15 @@
public function updateElementClasses( Element $element ) {
$classes = $this->getElementClasses( $element );
- $element
- ->removeClasses( $classes['off'] )
- ->addClasses( $classes['on'] );
+ if ( isset( $element->icon ) ) {
+ $element->icon
+ ->removeClasses( $classes['off'] )
+ ->addClasses( $classes['on'] );
+ }
+ if ( isset( $element->indicator ) ) {
+ $element->indicator
+ ->removeClasses( $classes['off'] )
+ ->addClasses( $classes['on'] );
+ }
}
}
diff --git a/src/Theme.js b/src/Theme.js
index 92de72e..13ef5bb 100644
--- a/src/Theme.js
+++ b/src/Theme.js
@@ -40,9 +40,17 @@
* @return {Object.<string,string[]>} Categorized class names with `on` and
`off` lists
*/
OO.ui.Theme.prototype.updateElementClasses = function ( element ) {
- var classes = this.getElementClasses( element );
+ var $elements = $( [] ),
+ classes = this.getElementClasses( element );
- element.$element
+ if ( element.$icon ) {
+ $elements = $elements.add( element.$icon );
+ }
+ if ( element.$indicator ) {
+ $elements = $elements.add( element.$indicator );
+ }
+
+ $elements
.removeClass( classes.off.join( ' ' ) )
.addClass( classes.on.join( ' ' ) );
};
diff --git a/src/mixins/IconElement.js b/src/mixins/IconElement.js
index a4abe70..85e7f59 100644
--- a/src/mixins/IconElement.js
+++ b/src/mixins/IconElement.js
@@ -109,6 +109,8 @@
if ( this.iconTitle !== null ) {
this.$icon.attr( 'title', this.iconTitle );
}
+
+ this.updateThemeClasses();
};
/**
diff --git a/src/mixins/IndicatorElement.js b/src/mixins/IndicatorElement.js
index 732780c..b8c04f2 100644
--- a/src/mixins/IndicatorElement.js
+++ b/src/mixins/IndicatorElement.js
@@ -90,6 +90,8 @@
if ( this.indicatorTitle !== null ) {
this.$indicator.attr( 'title', this.indicatorTitle );
}
+
+ this.updateThemeClasses();
};
/**
diff --git a/src/themes/mediawiki/icons.json b/src/themes/mediawiki/icons.json
index f351b5b..a79b329 100644
--- a/src/themes/mediawiki/icons.json
+++ b/src/themes/mediawiki/icons.json
@@ -1,6 +1,6 @@
{
"selectorWithoutVariant": ".oo-ui-icon-{name}",
- "selectorWithVariant": ".oo-ui-image-{variant} .oo-ui-icon-{name},
.oo-ui-image-{variant}.oo-ui-icon-{name}",
+ "selectorWithVariant": ".oo-ui-image-{variant}.oo-ui-icon-{name}",
"intro": "@import '../../../../src/styles/common';",
"variants": {
"invert": {
diff --git a/src/themes/mediawiki/indicators.json
b/src/themes/mediawiki/indicators.json
index 3d66337..5a83258 100644
--- a/src/themes/mediawiki/indicators.json
+++ b/src/themes/mediawiki/indicators.json
@@ -1,6 +1,6 @@
{
"selectorWithoutVariant": ".oo-ui-indicator-{name}",
- "selectorWithVariant": ".oo-ui-image-{variant} .oo-ui-indicator-{name},
.oo-ui-image-{variant}.oo-ui-indicator-{name}",
+ "selectorWithVariant": ".oo-ui-image-{variant}.oo-ui-indicator-{name}",
"intro": "@import '../../../../src/styles/common';",
"variants": {
"invert": {
--
To view, visit https://gerrit.wikimedia.org/r/231602
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie93e4d6ed5637cdd95367729e1349a6da923f461
Gerrit-PatchSet: 8
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Mooeypoo <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
