Trevor Parscal has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/178595

Change subject: [IDEA] Make icon prefix theme-configurable
......................................................................

[IDEA] Make icon prefix theme-configurable

And use mw-ui for the MediaWiki theme

Pros: OOjs UI can be integrated into an existing software ecosystem (like 
MediaWiki) and share icons already present or common with other non-OOjs UI 
components.

Cons: All software that uses OOjs UI (like VisualEditor) would need to add 
icons to client using the correct prefix as defined by the theme, which might 
be akward or just plain stupid.

Bonus (cleanup that this patch includes):
There was a very confusing configuration of "css" ... "classPrefix" which was 
actually a less mixin, and not always a prefix at all. Even more confusing, an 
empty string fallback was in place which would have crashed LESS if it were 
ever used and incorrect documentation made the whole situation all that much 
worse. Now less mixins are configured explicitly.

Also, within the mixins, the output class prefixes would (by default) collide 
with the naming of the mixins, which didn't actually cause any errors, until 
you tried to read it and then you would easily get very confused. Now that we 
are fully parameterizing the prefixes of all CSS rules for images these are 
much easier to understand.

Change-Id: I3d9d5356b7b455251a0f11c24a286500f8b245e9
---
M build/images.json
M build/tasks/colorize-svg.js
M demos/demo.js
M php/Theme.php
M php/elements/IconElement.php
M php/elements/IndicatorElement.php
M php/themes/MediaWikiTheme.php
M src/Theme.js
M src/elements/IconElement.js
M src/elements/IndicatorElement.js
M src/styles/common.less
M src/themes/mediawiki/MediaWikiTheme.js
M src/themes/mediawiki/common.less
13 files changed, 105 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/95/178595/1

diff --git a/build/images.json b/build/images.json
index e522a14..8f47302 100644
--- a/build/images.json
+++ b/build/images.json
@@ -1,13 +1,16 @@
 {
-       "css": {
+       "mixins": {
                "indicators": {
-                       "classPrefix": ".oo-ui-indicator"
+                       "image": ".oo-ui-indicator",
+                       "variant": ".oo-ui-indicator-variant"
                },
                "icons": {
-                       "classPrefix": ".oo-ui-icon"
+                       "image": ".oo-ui-icon",
+                       "variant": ".oo-ui-icon-variant"
                },
                "textures": {
-                       "classPrefix": ".oo-ui-texture"
+                       "image": ".oo-ui-texture",
+                       "variant": ".oo-ui-texture-variant"
                }
        }
 }
diff --git a/build/tasks/colorize-svg.js b/build/tasks/colorize-svg.js
index 0ac2f79..32a8d7a 100644
--- a/build/tasks/colorize-svg.js
+++ b/build/tasks/colorize-svg.js
@@ -27,7 +27,7 @@
                                        data.srcDir,
                                        options.images,
                                        options.variants,
-                                       options.css
+                                       options.mixins
                                ),
                                imageLists = source.getImageLists();
 
@@ -59,13 +59,13 @@
         * @param {string} path Directory containing source images
         * @param {Object} images Lists of image configurations, keyed by type
         * @param {Object} variants List of variant configurations, keyed by 
type and variant name
-        * @param {Object} css List of CSS class configurations, keyed by type
+        * @param {Object} mixins List of LESS mixin configurations, keyed by 
type
         */
-       function Source( path, images, variants, css ) {
+       function Source( path, images, variants, mixins ) {
                this.path = path;
                this.images = images;
                this.variants = variants;
-               this.css = css;
+               this.mixins = mixins;
        }
 
        /**
@@ -90,7 +90,7 @@
                        lists[type] = new ImageList(
                                path.join( this.path, type ),
                                new VariantList( this.variants[type] || {} ),
-                               this.css[type] || {},
+                               this.mixins[type] || {},
                                this.images[type]
                        );
                }
@@ -168,7 +168,8 @@
                        filePath = path.join( this.list.getPath(), file ),
                        variable = fileExtensionBase.toLowerCase() === 'svg',
                        variants = this.list.getVariants(),
-                       cssClassPrefix = this.list.getCssClassPrefix(),
+                       imageMixin = this.list.getImageMixin(),
+                       variantMixin = this.list.getVariantMixin(),
                        rules = [],
                        uncolorizableImages = [],
                        unknownVariants = [];
@@ -179,7 +180,7 @@
                        params.push( fileExtensionBase );
                }
                grunt.file.copy( filePath, path.join( destination.getPath(), 
file ) );
-               rules.push( cssClassPrefix + '( ' + params.join( ', ' ) + ' );' 
);
+               rules.push( imageMixin + '( ' + params.join( ', ' ) + ' );' );
 
                // Variants
                if ( variable ) {
@@ -213,7 +214,7 @@
                                        ),
                                        variantSvg
                                );
-                               return cssClassPrefix + '-variant' + '( ' + 
params.join( ', ' ) + ' );';
+                               return variantMixin + '( ' + params.join( ', ' 
) + ' );';
                        } ) );
                }
 
@@ -248,16 +249,16 @@
         * @constructor
         * @param {string} path Images path
         * @param {VariantList} variants Variants list
-        * @param {Object} css CSS class configuration
+        * @param {Object} mixins List of LESS mixin configurations, containing 
`image` and `variant`
         * @param {Object} data List of image configurations keyed by name
         */
-       function ImageList( path, variants, css, data ) {
+       function ImageList( path, variants, mixins, data ) {
                var key;
 
                this.list = {};
                this.path = path;
                this.variants = variants;
-               this.css = css;
+               this.mixins = mixins;
 
                for ( key in data ) {
                        this.list[key] = new Image( this, key, data[key] );
@@ -283,12 +284,21 @@
        };
 
        /**
-        * Get CSS class configuration.
+        * Get LESS mixin for generating images.
         *
-        * @return {VariantsList} CSS class configuration
+        * @return {string} LESS mixin name
         */
-       ImageList.prototype.getCssClassPrefix = function () {
-               return this.css.classPrefix || '';
+       ImageList.prototype.getImageMixin = function () {
+               return this.mixins.image;
+       };
+
+       /**
+        * Get LESS mixin name for generating variants.
+        *
+        * @return {string} LESS mixin name
+        */
+       ImageList.prototype.getVariantMixin = function () {
+               return this.mixins.variant;
        };
 
        /**
diff --git a/demos/demo.js b/demos/demo.js
index 22b6e91..c4677c7 100644
--- a/demos/demo.js
+++ b/demos/demo.js
@@ -11,9 +11,12 @@
        // Normalization
        this.normalizeHash();
 
+       // Theme setup
+       this.mode = this.getCurrentMode();
+       OO.ui.theme = new ( 
this.constructor.static.themes[this.mode.theme].theme )();
+
        // Properties
        this.stylesheetLinks = this.getStylesheetLinks();
-       this.mode = this.getCurrentMode();
        this.$menu = this.$( '<div>' );
        this.pageDropdown = new OO.ui.DropdownWidget( {
                $: this.$,
@@ -66,7 +69,6 @@
        $( 'body' ).addClass( 'oo-ui-' + this.mode.direction );
        $( 'head' ).append( this.stylesheetLinks );
        this.constructor.static.pages[this.mode.page]( this );
-       OO.ui.theme = new ( 
this.constructor.static.themes[this.mode.theme].theme )();
 };
 
 /* Setup */
diff --git a/php/Theme.php b/php/Theme.php
index 39c266b..bac0654 100644
--- a/php/Theme.php
+++ b/php/Theme.php
@@ -26,6 +26,15 @@
        }
 
        /**
+        * Get a prefix for image CSS classes.
+        *
+        * @return string CSS class prefix
+        */
+       public function getImageClassPrefix() {
+               return 'oo-ui';
+       }
+
+       /**
         * Get a list of classes to be applied to a widget.
         *
         * The 'on' and 'off' lists combined MUST contain keys for all classes 
the theme adds or removes,
diff --git a/php/elements/IconElement.php b/php/elements/IconElement.php
index 587b559..15cb9ed 100644
--- a/php/elements/IconElement.php
+++ b/php/elements/IconElement.php
@@ -18,6 +18,13 @@
         */
        protected $icon = null;
 
+       /**
+        * CSS class prefix
+        *
+        * @var string|null
+        */
+       protected $prefix = null;
+
        public static $targetPropertyName = 'icon';
 
        /**
@@ -32,6 +39,7 @@
                parent::__construct( $element, $target, $config );
 
                // Initialization
+               $this->prefix = Theme::singleton()->getImageClassPrefix() . 
'-icon-';
                $this->target->addClasses( array( 'oo-ui-iconElement-icon' ) );
                $this->setIcon( isset( $config['icon'] ) ? $config['icon'] : 
null );
        }
@@ -44,10 +52,10 @@
         */
        public function setIcon( $icon = null ) {
                if ( $this->icon !== null ) {
-                       $this->target->removeClasses( array( 'oo-ui-icon-' . 
$this->icon ) );
+                       $this->target->removeClasses( array( $this->prefix . 
$this->icon ) );
                }
                if ( $icon !== null ) {
-                       $this->target->addClasses( array( 'oo-ui-icon-' . $icon 
) );
+                       $this->target->addClasses( array( $this->prefix . $icon 
) );
                }
 
                $this->icon = $icon;
diff --git a/php/elements/IndicatorElement.php 
b/php/elements/IndicatorElement.php
index 876e75f..4b49366 100644
--- a/php/elements/IndicatorElement.php
+++ b/php/elements/IndicatorElement.php
@@ -18,6 +18,13 @@
         */
        protected $indicator = null;
 
+       /**
+        * CSS class prefix
+        *
+        * @var string|null
+        */
+       protected $prefix = null;
+
        public static $targetPropertyName = 'indicator';
 
        /**
@@ -34,6 +41,7 @@
                parent::__construct( $element, $target, $config );
 
                // Initialization
+               $this->prefix = Theme::singleton()->getImageClassPrefix() . 
'-indicator-';
                $this->target->addClasses( array( 
'oo-ui-indicatorElement-indicator' ) );
                $this->setIndicator( isset( $config['indicator'] ) ? 
$config['indicator'] : null );
        }
@@ -46,10 +54,10 @@
         */
        public function setIndicator( $indicator = null ) {
                if ( $this->indicator !== null ) {
-                       $this->target->removeClasses( array( 'oo-ui-indicator-' 
. $this->indicator ) );
+                       $this->target->removeClasses( array( $this->prefix . 
$this->indicator ) );
                }
                if ( $indicator !== null ) {
-                       $this->target->addClasses( array( 'oo-ui-indicator-' . 
$indicator ) );
+                       $this->target->addClasses( array( $this->prefix . 
$indicator ) );
                }
 
                $this->indicator = $indicator;
diff --git a/php/themes/MediaWikiTheme.php b/php/themes/MediaWikiTheme.php
index 54ff8af..c5afe70 100644
--- a/php/themes/MediaWikiTheme.php
+++ b/php/themes/MediaWikiTheme.php
@@ -6,6 +6,10 @@
 
        /* Methods */
 
+       public function getImageClassPrefix() {
+               return 'mw-ui';
+       }
+
        public function getElementClasses( Element $element ) {
                $variants = array(
                        'invert' => false,
@@ -34,7 +38,7 @@
                }
 
                foreach ( $variants as $variant => $toggle ) {
-                       $classes[$toggle ? 'on' : 'off'][] = 'oo-ui-image-' . 
$variant;
+                       $classes[$toggle ? 'on' : 'off'][] = 'mw-ui-image-' . 
$variant;
                }
 
                return $classes;
diff --git a/src/Theme.js b/src/Theme.js
index 92de72e..2cb6737 100644
--- a/src/Theme.js
+++ b/src/Theme.js
@@ -19,6 +19,16 @@
 /* Methods */
 
 /**
+ * Get a prefix for image CSS classes.
+ *
+ * @localdoc Always returns the default value: 'oo-ui'
+ * @return {string} CSS class prefix
+ */
+OO.ui.Theme.prototype.getImageClassPrefix = function () {
+       return 'oo-ui';
+};
+
+/**
  * Get a list of classes to be applied to a widget.
  *
  * The 'on' and 'off' lists combined MUST contain keys for all classes the 
theme adds or removes,
diff --git a/src/elements/IconElement.js b/src/elements/IconElement.js
index 2f2e97b..b920417 100644
--- a/src/elements/IconElement.js
+++ b/src/elements/IconElement.js
@@ -25,6 +25,7 @@
        this.$icon = null;
        this.icon = null;
        this.iconTitle = null;
+       this.prefix = OO.ui.theme.getImageClassPrefix() + '-icon-';
 
        // Initialization
        this.setIcon( config.icon || this.constructor.static.icon );
@@ -79,13 +80,13 @@
 OO.ui.IconElement.prototype.setIconElement = function ( $icon ) {
        if ( this.$icon ) {
                this.$icon
-                       .removeClass( 'oo-ui-iconElement-icon oo-ui-icon-' + 
this.icon )
+                       .removeClass( 'oo-ui-iconElement-icon ' + this.prefix + 
this.icon )
                        .removeAttr( 'title' );
        }
 
        this.$icon = $icon
                .addClass( 'oo-ui-iconElement-icon' )
-               .toggleClass( 'oo-ui-icon-' + this.icon, !!this.icon );
+               .toggleClass( this.prefix + this.icon, !!this.icon );
        if ( this.iconTitle !== null ) {
                this.$icon.attr( 'title', this.iconTitle );
        }
@@ -106,10 +107,10 @@
        if ( this.icon !== icon ) {
                if ( this.$icon ) {
                        if ( this.icon !== null ) {
-                               this.$icon.removeClass( 'oo-ui-icon-' + 
this.icon );
+                               this.$icon.removeClass( this.prefix + this.icon 
);
                        }
                        if ( icon !== null ) {
-                               this.$icon.addClass( 'oo-ui-icon-' + icon );
+                               this.$icon.addClass( this.prefix + icon );
                        }
                }
                this.icon = icon;
diff --git a/src/elements/IndicatorElement.js b/src/elements/IndicatorElement.js
index 327c6b4..3940ec5 100644
--- a/src/elements/IndicatorElement.js
+++ b/src/elements/IndicatorElement.js
@@ -24,6 +24,7 @@
        this.$indicator = null;
        this.indicator = null;
        this.indicatorTitle = null;
+       this.prefix = OO.ui.theme.getImageClassPrefix() + '-indicator-';
 
        // Initialization
        this.setIndicator( config.indicator || 
this.constructor.static.indicator );
@@ -68,13 +69,13 @@
 OO.ui.IndicatorElement.prototype.setIndicatorElement = function ( $indicator ) 
{
        if ( this.$indicator ) {
                this.$indicator
-                       .removeClass( 'oo-ui-indicatorElement-indicator 
oo-ui-indicator-' + this.indicator )
+                       .removeClass( 'oo-ui-indicatorElement-indicator ' + 
this.prefix + this.indicator )
                        .removeAttr( 'title' );
        }
 
        this.$indicator = $indicator
                .addClass( 'oo-ui-indicatorElement-indicator' )
-               .toggleClass( 'oo-ui-indicator-' + this.indicator, 
!!this.indicator );
+               .toggleClass( this.prefix + this.indicator, !!this.indicator );
        if ( this.indicatorTitle !== null ) {
                this.$indicatorTitle.attr( 'title', this.indicatorTitle );
        }
@@ -92,10 +93,10 @@
        if ( this.indicator !== indicator ) {
                if ( this.$indicator ) {
                        if ( this.indicator !== null ) {
-                               this.$indicator.removeClass( 'oo-ui-indicator-' 
+ this.indicator );
+                               this.$indicator.removeClass( this.prefix + 
this.indicator );
                        }
                        if ( indicator !== null ) {
-                               this.$indicator.addClass( 'oo-ui-indicator-' + 
indicator );
+                               this.$indicator.addClass( this.prefix + 
indicator );
                        }
                }
                this.indicator = indicator;
diff --git a/src/styles/common.less b/src/styles/common.less
index d0dc08a..be3a7cf 100644
--- a/src/styles/common.less
+++ b/src/styles/common.less
@@ -4,6 +4,8 @@
 
 // @oo-ui-default-image-ext set during build
 @oo-ui-default-image-path: 'images';
+@oo-ui-image-class-prefix: oo-ui;
+@oo-ui-image-variant-class-prefix: oo-ui-image;
 
 // Mixins
 
@@ -16,14 +18,14 @@
 }
 
 .oo-ui-image( @type, @name, @file, @ext, @path ) {
-       .oo-ui-@{type}-@{name} {
+       .@{oo-ui-image-class-prefix}-@{type}-@{name} {
                .oo-ui-background-image('@{path}/@{type}s/@{file}.@{ext}');
        }
 }
 
 .oo-ui-image-variant( @type, @name, @file, @variant, @ext, @path ) {
-       .oo-ui-image-@{variant} .oo-ui-@{type}-@{name},
-       .oo-ui-image-@{variant}.oo-ui-@{type}-@{name} {
+       .@{oo-ui-image-variant-class-prefix}-@{variant} 
.@{oo-ui-image-class-prefix}-@{type}-@{name},
+       
.@{oo-ui-image-variant-class-prefix}-@{variant}.@{oo-ui-image-class-prefix}-@{type}-@{name}
 {
                
.oo-ui-background-image('@{path}/@{type}s/@{file}-@{variant}.@{ext}');
        }
 }
diff --git a/src/themes/mediawiki/MediaWikiTheme.js 
b/src/themes/mediawiki/MediaWikiTheme.js
index 31a7d34..4b2502d 100644
--- a/src/themes/mediawiki/MediaWikiTheme.js
+++ b/src/themes/mediawiki/MediaWikiTheme.js
@@ -18,6 +18,13 @@
 /**
  * @inheritdoc
  */
+OO.ui.MediaWikiTheme.prototype.getImageClassPrefix = function () {
+       return 'mw-ui';
+};
+
+/**
+ * @inheritdoc
+ */
 OO.ui.MediaWikiTheme.prototype.getElementClasses = function ( element ) {
        // Parent method
        var variant,
@@ -47,7 +54,7 @@
        }
 
        for ( variant in variants ) {
-               classes[variants[variant] ? 'on' : 'off'].push( 'oo-ui-image-' 
+ variant );
+               classes[variants[variant] ? 'on' : 'off'].push( 'mw-ui-image-' 
+ variant );
        }
 
        return classes;
diff --git a/src/themes/mediawiki/common.less b/src/themes/mediawiki/common.less
index 70ff340..03423c0 100644
--- a/src/themes/mediawiki/common.less
+++ b/src/themes/mediawiki/common.less
@@ -34,6 +34,8 @@
 @destructive-fill-selected: #a7170f;
 
 @oo-ui-default-image-path: 'themes/mediawiki/images';
+@oo-ui-image-class-prefix: mw-ui;
+@oo-ui-image-variant-class-prefix: mw-ui-image;
 
 @checkbox-size: 1.6em;
 @border-radius: 0.3em;

-- 
To view, visit https://gerrit.wikimedia.org/r/178595
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d9d5356b7b455251a0f11c24a286500f8b245e9
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to