MDL-40759 icons: Refactor to allow theme icon systems
authorDamyon Wiese <damyon@moodle.com>
Thu, 2 Feb 2017 04:59:55 +0000 (12:59 +0800)
committerDamyon Wiese <damyon@moodle.com>
Fri, 17 Mar 2017 07:52:18 +0000 (15:52 +0800)
14 files changed:
lib/amd/build/icon_system.min.js
lib/amd/build/icon_system_fontawesome.min.js
lib/amd/build/templates.min.js
lib/amd/src/icon_system.js
lib/amd/src/icon_system_fontawesome.js
lib/amd/src/templates.js
lib/classes/output/external.php
lib/classes/output/icon_system.php
lib/classes/output/icon_system_font.php
lib/classes/output/icon_system_fontawesome.php
lib/db/services.php
lib/outputrenderers.php
lib/outputrequirementslib.php
theme/boost/config.php

index c3aa058..e30f717 100644 (file)
Binary files a/lib/amd/build/icon_system.min.js and b/lib/amd/build/icon_system.min.js differ
index 73a476e..b6d265d 100644 (file)
Binary files a/lib/amd/build/icon_system_fontawesome.min.js and b/lib/amd/build/icon_system_fontawesome.min.js differ
index 0e236ed..6a0a867 100644 (file)
Binary files a/lib/amd/build/templates.min.js and b/lib/amd/build/templates.min.js differ
index 963e8a4..1d28046 100644 (file)
@@ -26,7 +26,7 @@ define(['jquery'], function($) {
     /**
      * Icon System abstract class.
      *
-     * Any icon system needs to define a module extending this one with the name core/icon_system_blah.
+     * Any icon system needs to define a module extending this one and return this module name from the php icon_system class.
      */
     var IconSystem = function() {
     };
@@ -38,12 +38,18 @@ define(['jquery'], function($) {
      * @method init
      */
     IconSystem.prototype.init = function() {
-        return $.when();
+        return $.when(this);
     };
 
     /**
      * Render an icon.
      *
+     * The key, component and title come from either the pix mustache helper tag, or the call to templates.renderIcon.
+     * The template is the pre-loaded template string matching the template from getTemplateName() in this class.
+     * This function must return a string (not a promise) because it is used during the internal rendering of the mustache
+     * template (which is unfortunately synchronous). To render the mustache template in this function call
+     * core/mustache.render() directly and do not use any partials, blocks or helper functions in the template.
+     *
      * @param {String} key
      * @param {String} component
      * @param {String} title
@@ -55,5 +61,15 @@ define(['jquery'], function($) {
         throw new Error('Abstract function not implemented.');
     };
 
+    /**
+     * getTemplateName
+     *
+     * @return {String}
+     * @method getTemplateName
+     */
+    IconSystem.prototype.getTemplateName = function() {
+        throw new Error('Abstract function not implemented.');
+    };
+
     return /** @alias module:core/icon_system */ IconSystem;
 });
index 3adbb70..49258a7 100644 (file)
@@ -35,9 +35,15 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
     };
     IconSystemFontawesome.prototype = Object.create(IconSystem.prototype);
 
+    /**
+     * Prefetch resources so later calls to renderIcon can be resolved synchronously.
+     *
+     * @method init
+     * @return Promise
+     */
     IconSystemFontawesome.prototype.init = function() {
         if (staticMap) {
-            return $.when();
+            return $.when(this);
         }
 
         var map = LocalStorage.get('core/iconmap-fontawesome');
@@ -47,13 +53,13 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
 
         if (map) {
             staticMap = map;
-            return $.when();
+            return $.when(this);
         }
 
         if (fetchMap === null) {
             fetchMap = Ajax.call([{
-                methodname: 'core_output_load_icon_map',
-                args: { 'system': 'fontawesome' }
+                methodname: 'core_output_load_fontawesome_icon_map',
+                args: []
             }], true, false)[0];
         }
 
@@ -63,6 +69,7 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
                 staticMap[value.component + '/' + value.pix] = value.to;
             }.bind(this));
             LocalStorage.set('core/iconmap-fontawesome', JSON.stringify(staticMap));
+            return this;
         }.bind(this));
     };
 
@@ -90,6 +97,16 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
         return Mustache.render(template, context);
     };
 
+    /**
+     * Get the name of the template to pre-cache for this icon system.
+     *
+     * @return {String}
+     * @method getTemplateName
+     */
+    IconSystemFontawesome.prototype.getTemplateName = function() {
+        return 'core/pix_icon_fontawesome';
+    };
+
     return /** @alias module:core/icon_system_fontawesome */ IconSystemFontawesome;
 
 });
index 7e658dc..8cf4c9e 100644 (file)
@@ -160,26 +160,22 @@ define(['core/mustache',
      */
     Renderer.prototype.renderIcon = function(key, component, title) {
         // Preload the module to do the icon rendering based on the theme iconsystem.
-        var modulename = 'core/icon_system_' + config.iconsystem;
+        var modulename = config.iconsystemmodule;
 
+        // RequireJS does not return a promise.
         var ready = $.Deferred();
         require([modulename], function(System) {
             var system = new System();
             if (!(system instanceof IconSystem)) {
-                ready.reject('Invalid icon system specified' + config.iconsystem);
+                ready.reject('Invalid icon system specified' + config.iconsystemmodule);
             } else {
-                system.init().then(ready.resolve);
                 iconSystem = system;
+                system.init().then(ready.resolve);
             }
         });
 
-        var suffix = '';
-        if (config.iconsystem != 'standard') {
-            suffix = '_' + config.iconsystem;
-        }
-
-        return ready.then(function() {
-            return this.getTemplate('core/pix_icon' + suffix);
+        return ready.then(function(iconSystem) {
+            return this.getTemplate(iconSystem.getTemplateName());
         }.bind(this)).then(function(template) {
             return iconSystem.renderIcon(key, component, title, template);
         });
@@ -211,13 +207,9 @@ define(['core/mustache',
             text = helper(parts.join(',').trim());
         }
 
-        var suffix = '';
-        if (config.iconsystem != 'standard') {
-            suffix = '_' + config.iconsystem;
-        }
-
+        var templateName = iconSystem.getTemplateName();
 
-        var searchKey = this.currentThemeName + '/core/pix_icon' + suffix;
+        var searchKey = this.currentThemeName + '/' + templateName;
         var template = templateCache[searchKey];
 
         return iconSystem.renderIcon(key, component, text, template);
@@ -508,12 +500,9 @@ define(['core/mustache',
      */
     Renderer.prototype.doRender = function(templateSource, context, themeName) {
         this.currentThemeName = themeName;
-        var suffix = '';
-        if (config.iconsystem != 'standard') {
-            suffix = '_' + config.iconsystem;
-        }
+        var iconTemplate = iconSystem.getTemplateName();
 
-        return this.getTemplate('core/pix_icon' + suffix).then(function() {
+        return this.getTemplate(iconTemplate).then(function() {
             this.addHelpers(context, themeName);
             var result = mustache.render(templateSource, context, this.partialHelper.bind(this));
             return $.Deferred().resolve(result.trim(), this.getJS()).promise();
@@ -695,7 +684,7 @@ define(['core/mustache',
         this.currentThemeName = themeName;
 
         // Preload the module to do the icon rendering based on the theme iconsystem.
-        var modulename = 'core/icon_system_' + config.iconsystem;
+        var modulename = config.iconsystemmodule;
 
         var ready = $.Deferred();
         require([modulename], function(System) {
index a57d096..132ff1c 100644 (file)
@@ -100,10 +100,8 @@ class external extends external_api {
      *
      * @return external_function_parameters
      */
-    public static function load_icon_map_parameters() {
-        return new external_function_parameters([
-                'system' => new external_value(PARAM_COMPONENT, 'icon system to load the map for')
-        ]);
+    public static function load_fontawesome_icon_map_parameters() {
+        return new external_function_parameters([]);
     }
 
     /**
@@ -112,17 +110,9 @@ class external extends external_api {
      * @param string $system
      * @return array the mapping
      */
-    public static function load_icon_map($system) {
-        $params = self::validate_parameters(self::load_icon_map_parameters(),
-                                            array('system' => $system));
-
-        $system = $params['system'];
-
-        if (!icon_system::is_valid_system($system)) {
-            throw new coding_exception('Invalid icon system.');
-        }
+    public static function load_fontawesome_icon_map() {
         
-        $instance = icon_system::instance($system);
+        $instance = icon_system::instance(icon_system::FONTAWESOME);
 
         $map = $instance->get_icon_name_map();
 
@@ -144,7 +134,7 @@ class external extends external_api {
      *
      * @return external_description
      */
-    public static function load_icon_map_returns() {
+    public static function load_fontawesome_icon_map_returns() {
         return new external_multiple_structure(new external_single_structure(
             array(
                 'component' => new external_value(PARAM_COMPONENT, 'The component for the icon.'),
index e8b1211..6c132c2 100644 (file)
@@ -42,17 +42,13 @@ use pix_icon;
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 abstract class icon_system {
-    const STANDARD = 'standard';
-    const FONTAWESOME = 'fontawesome';
-    const INLINE = 'inline';
-    const SYSTEMS = [ self::STANDARD, self::FONTAWESOME, self::INLINE ];
+    const STANDARD = '\\core\\output\\icon_system_standard';
+    const FONTAWESOME = '\\core\\output\\icon_system_fontawesome';
 
     private static $instance = null;
     private $map = null;
-    protected $system = '';
 
-    private function __construct($system) {
-        $this->system = $system;
+    private function __construct() {
     }
 
     public final static function instance($type = null) {
@@ -63,14 +59,12 @@ abstract class icon_system {
                 return self::$instance;
             }
             $type = $PAGE->theme->get_icon_system();
-            $system = '\\core\\output\\icon_system_' . $type;
-            self::$instance = new $system($type);
+            self::$instance = new $type();
             // Default one is a singleton.
             return self::$instance;
         } else {
-            $system = '\\core\\output\\icon_system_' . $type;
             // Not a singleton.
-            return new $system($type);
+            return new $type();
         }
     }
 
@@ -81,9 +75,16 @@ abstract class icon_system {
      * @return boolean
      */
     public final static function is_valid_system($system) {
-        return in_array($system, self::SYSTEMS);
+        return class_exists($system) && is_subclass_of($system, self::class);
     }
 
+    /**
+     * The name of an AMD module extending core/icon_system
+     *
+     * @return string
+     */
+    public abstract function get_amd_name();
+
     /**
      * Render the pix icon according to the icon system.
      *
index 6900df4..71c8b7a 100644 (file)
@@ -25,9 +25,6 @@
 
 namespace core\output;
 
-use renderer_base;
-use pix_icon;
-
 /**
  * Class allowing different systems for mapping and rendering icons.
  *
@@ -43,41 +40,5 @@ use pix_icon;
  */
 abstract class icon_system_font extends icon_system {
 
-    private $map = [];
-
-    public function get_core_icon_map() {
-        return [];
-    }
-
-    public function render_pix_icon(renderer_base $output, pix_icon $icon) {
-        $subtype = 'pix_icon_' . $this->system;
-        $subpix = new $subtype($icon);
-        $data = $subpix->export_for_template($output);
-
-        return $output->render_from_template('core/pix_icon_' . $this->system, $data);
-    }
-
-    /**
-     * Overridable function to get a mapping of all icons.
-     * Default is to do no mapping.
-     */
-    public function get_icon_name_map() {
-        if ($this->map === []) {
-            $this->map = $this->get_core_icon_map();
-            $callback = 'get_' . $this->system . '_icon_map';
-
-            if ($pluginsfunction = get_plugins_with_function($callback)) {
-                foreach ($pluginsfunction as $plugintype => $plugins) {
-                    foreach ($plugins as $pluginfunction) {
-                        $pluginmap = $pluginfunction();
-                        $this->map += $pluginmap;
-                    }
-                }
-            }
-
-        }
-        return $this->map;
-    }
-
 }
 
index b83fb1b..444ae01 100644 (file)
@@ -25,6 +25,9 @@
 
 namespace core\output;
 
+use renderer_base;
+use pix_icon;
+
 /**
  * Class allowing different systems for mapping and rendering icons.
  *
@@ -414,5 +417,43 @@ class icon_system_fontawesome extends icon_system_font {
             'core:t/viewdetails' => 'fa-list',
         ];
     }
+
+    private $map = [];
+
+    /**
+     * Overridable function to get a mapping of all icons.
+     * Default is to do no mapping.
+     */
+    public function get_icon_name_map() {
+        if ($this->map === []) {
+            $this->map = $this->get_core_icon_map();
+            $callback = 'get_fontawesome_icon_map';
+
+            if ($pluginsfunction = get_plugins_with_function($callback)) {
+                foreach ($pluginsfunction as $plugintype => $plugins) {
+                    foreach ($plugins as $pluginfunction) {
+                        $pluginmap = $pluginfunction();
+                        $this->map += $pluginmap;
+                    }
+                }
+            }
+
+        }
+        return $this->map;
+    }
+
+
+    public function get_amd_name() {
+        return 'core/icon_system_fontawesome';
+    }
+
+    public function render_pix_icon(renderer_base $output, pix_icon $icon) {
+        $subtype = 'pix_icon_fontawesome';
+        $subpix = new $subtype($icon);
+        $data = $subpix->export_for_template($output);
+
+        return $output->render_from_template('core/pix_icon_fontawesome', $data);
+    }
+
 }
 
index aa13333..5ff30cd 100644 (file)
@@ -945,9 +945,9 @@ $functions = array(
         'loginrequired' => false,
         'ajax' => true,
     ),
-    'core_output_load_icon_map' => array(
+    'core_output_load_fontawesome_icon_map' => array(
         'classname' => 'core\output\external',
-        'methodname' => 'load_icon_map',
+        'methodname' => 'load_fontawesome_icon_map',
         'description' => 'Load the mapping of names to icons',
         'type' => 'read',
         'loginrequired' => false,
index 94df7fd..181cb1c 100644 (file)
@@ -2054,7 +2054,7 @@ class core_renderer extends renderer_base {
     protected function render_activity_icon(activity_icon $icon) {
         global $PAGE;
 
-        $system = \core\output\icon_system::instance('standard');
+        $system = \core\output\icon_system::instance(\core\output\icon_system::STANDARD);
         return $system->render_pix_icon($this, $icon);
     }
 
index ab8c7ea..1406e40 100644 (file)
@@ -313,13 +313,15 @@ class page_requirements_manager {
             // Otherwise, in some situations, users will get warnings about insecure content
             // on secure pages from their web browser.
 
+            $iconsystem = \core\output\icon_system::instance();
+
             $this->M_cfg = array(
                 'wwwroot'             => $CFG->httpswwwroot, // Yes, really. See above.
                 'sesskey'             => sesskey(),
                 'themerev'            => theme_get_revision(),
                 'slasharguments'      => (int)(!empty($CFG->slasharguments)),
                 'theme'               => $page->theme->name,
-                'iconsystem'          => $page->theme->get_icon_system(),
+                'iconsystemmodule'    => $iconsystem->get_amd_name(),
                 'jsrev'               => $this->get_jsrev(),
                 'admin'               => $CFG->admin,
                 'svgicons'            => $page->theme->use_svg_icons(),
index c1e32df..57e81c4 100644 (file)
@@ -153,4 +153,4 @@ $THEME->yuicssmodules = array();
 $THEME->rendererfactory = 'theme_overridden_renderer_factory';
 $THEME->requiredblocks = '';
 $THEME->addblockposition = BLOCK_ADDBLOCK_POSITION_FLATNAV;
-$THEME->iconsystem = 'fontawesome';
+$THEME->iconsystem = \core\output\icon_system::FONTAWESOME;