Skip to content

Commit

Permalink
MDL-40759 icons: Peer review fixes (all minor)
Browse files Browse the repository at this point in the history
  • Loading branch information
Damyon Wiese committed Mar 17, 2017
1 parent 1caabd8 commit b9b409c
Show file tree
Hide file tree
Showing 42 changed files with 131 additions and 85 deletions.
2 changes: 1 addition & 1 deletion admin/tool/lp/templates/comment_area.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
<div class="mdl-left">
{{^notoggle}}
<a href="#" class="comment-link" id="comment-link-{{cid}}">
{{#pix}}{{collapsediconkey}}, {{linktext}}{{/pix}}<span id="comment-link-text-{{cid}}">{{linktext}}
{{#pix}}{{collapsediconkey}}, core, {{linktext}}{{/pix}}<span id="comment-link-text-{{cid}}">{{linktext}}
{{#displaytotalcount}}
({{count}})
{{/displaytotalcount}}</span>
Expand Down
2 changes: 1 addition & 1 deletion admin/tool/usertours/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,6 @@ function tool_usertours_get_fontawesome_icon_map() {
return [
'tool_usertours:t/export' => 'fa-download',
'tool_usertours:i/reload' => 'fa-refresh',
'tool_usertours:filler' => 'fa-spacer',
'tool_usertours:t/filler' => 'fa-spacer',
];
}
2 changes: 1 addition & 1 deletion admin/user.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
} else {
$columnicon = ($dir == "ASC") ? "sort_asc" : "sort_desc";
}
$columnicon = $OUTPUT->pix_icon('t/' . $columnicon, '', 'core', ['class' => 'iconsort']);
$columnicon = $OUTPUT->pix_icon('t/' . $columnicon, get_string($columndir), 'core', ['class' => 'iconsort']);

}
$$column = "<a href=\"user.php?sort=$column&amp;dir=$columndir\">".$string[$column]."</a>$columnicon";
Expand Down
3 changes: 1 addition & 2 deletions admin/user/user_bulk_cohortadd.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,9 @@ function sort_compare($a, $b) {
} else {
$columndir = ($dir == 'asc') ? 'desc' : 'asc';
$icon = 't/down';
$iconstr = 'movedown';
$iconstr = $columndir;
if ($dir != 'asc') {
$icon = 't/up';
$iconstr = 'moveup';
}
$columnicon = ' ' . $OUTPUT->pix_icon($icon, get_string($iconstr));
}
Expand Down
3 changes: 1 addition & 2 deletions admin/user/user_bulk_display.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ function sort_compare($a, $b) {
} else {
$columndir = $dir == 'asc' ? 'desc' : 'asc';
$icon = 't/down';
$iconstr = 'movedown';
$iconstr = $columndir;
if ($dir != 'asc') {
$icon = 't/up';
$iconstr = 'moveup';
}
$columnicon = ' ' . $OUTPUT->pix_icon($icon, get_string($iconstr));
}
Expand Down
3 changes: 2 additions & 1 deletion blocks/course_overview/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ public function course_overview($courses, $overviews) {
$moveurl = new moodle_url('/blocks/course_overview/move.php',
array('sesskey' => sesskey(), 'moveto' => 0, 'courseid' => $movingcourseid));
// Create move icon, so it can be used.
$movetofirsticon = $this->output->pix_icon('movehere', get_string('movetofirst', 'block_course_overview', $courses[$movingcourseid]->fullname));
$name = $courses[$movingcourseid]->fullname;
$movetofirsticon = $this->output->pix_icon('movehere', get_string('movetofirst', 'block_course_overview', $name));
$moveurl = html_writer::link($moveurl, $movetofirsticon);
$html .= html_writer::tag('div', $moveurl, array('class' => 'movehere'));
}
Expand Down
1 change: 0 additions & 1 deletion comment/comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ M.core_comment = {
Templates.renderPix('t/delete', 'core', M.util.get_string('deletecomment', 'moodle')).then(function(html) {
Y.all('div.comment-delete a').set('innerHTML', html);
}).catch(Notification.exception);

});
},
register_pagination: function() {
Expand Down
2 changes: 1 addition & 1 deletion course/format/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ protected function section_right_content($section, $course, $onsectionpage) {
* @return string HTML to output.
*/
protected function section_left_content($section, $course, $onsectionpage) {
$o = $this->output->spacer();
$o = '';

if ($section->section != 0) {
// Only in the non-general sections.
Expand Down
1 change: 0 additions & 1 deletion enrol/manual/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -730,4 +730,3 @@ public function edit_instance_validation($data, $files, $instance, $context) {
}

}

3 changes: 0 additions & 3 deletions files/classes/external/stored_file_exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ protected static function define_other_properties() {
'icon' => array(
'type' => PARAM_RAW,
),
'iconurl' => array(
'type' => PARAM_URL,
),
'timecreatedformatted' => array(
'type' => PARAM_RAW
),
Expand Down
2 changes: 1 addition & 1 deletion filter/emoticon/tests/filter_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function test_filter_emoticon_formats() {
$this->assertEquals($expected, $filter->filter('(grr)', $options));

// And texts matching target formats are filtered.
$expected = '<i class="icon fa s/angry fa-fw emoticon" aria-hidden="true" title=""></i><span class="sr-only"></span>';
$expected = '<img class="icon emoticon" alt="angry" title="angry" src="http://www.example.com/moodle/theme/image.php/_s/boost/core/1/s/angry" />';
$options = array('originalformat' => FORMAT_HTML); // Only FORMAT_HTML is filtered, see {@link testable_filter_emoticon}.
$this->assertEquals($expected, $filter->filter('(grr)', $options));
}
Expand Down
2 changes: 1 addition & 1 deletion lib/amd/build/icon_system_fontawesome.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/amd/src/icon_system.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ define(['jquery'], function($) {
* @return {String}
* @method renderIcon
*/
IconSystem.prototype.renderIcon = function(key, component, title, template) { //eslint-disable-line no-unused-vars
IconSystem.prototype.renderIcon = function(key, component, title, template) { // eslint-disable-line no-unused-vars
throw new Error('Abstract function not implemented.');
};

Expand Down
6 changes: 3 additions & 3 deletions lib/amd/src/icon_system_fontawesome.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
* Prefetch resources so later calls to renderIcon can be resolved synchronously.
*
* @method init
* @return Promise
* @return {Promise}
*/
IconSystemFontawesome.prototype.init = function() {
if (staticMap) {
Expand Down Expand Up @@ -67,7 +67,7 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
staticMap = {};
$.each(map, function(index, value) {
staticMap[value.component + '/' + value.pix] = value.to;
}.bind(this));
});
LocalStorage.set('core/iconmap-fontawesome', JSON.stringify(staticMap));
return this;
}.bind(this));
Expand All @@ -83,7 +83,7 @@ define(['core/icon_system', 'jquery', 'core/ajax', 'core/mustache', 'core/locals
* @return {String}
* @method renderIcon
*/
IconSystemFontawesome.prototype.renderIcon = function(key, component, title, template) { //eslint-disable-line no-unused-vars
IconSystemFontawesome.prototype.renderIcon = function(key, component, title, template) { // eslint-disable-line no-unused-vars
var mappedIcon = staticMap[component + '/' + key];
if (typeof mappedIcon === "undefined") {
mappedIcon = component + '/' + key;
Expand Down
2 changes: 1 addition & 1 deletion lib/amd/src/icon_system_standard.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ define(['core/icon_system', 'core/url', 'core/mustache'],
* @return {String}
* @method renderIcon
*/
IconSystemStandard.prototype.renderIcon = function(key, component, title, template) { //eslint-disable-line no-unused-vars
IconSystemStandard.prototype.renderIcon = function(key, component, title, template) { // eslint-disable-line no-unused-vars
var url = CoreUrl.imageUrl(key, component);

var templatecontext = {
Expand Down
2 changes: 0 additions & 2 deletions lib/classes/output/external.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,9 @@ public static function load_fontawesome_icon_map_parameters() {
/**
* Return a mapping of icon names to icons.
*
* @param string $system
* @return array the mapping
*/
public static function load_fontawesome_icon_map() {

$instance = icon_system::instance(icon_system::FONTAWESOME);

$map = $instance->get_icon_name_map();
Expand Down
24 changes: 24 additions & 0 deletions lib/classes/output/icon_system.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
use renderer_base;
use pix_icon;

defined('MOODLE_INTERNAL') || die();

/**
* Class allowing different systems for mapping and rendering icons.
*
Expand All @@ -42,15 +44,37 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
abstract class icon_system {
/**
* @const STANDARD Default icon system.
*/
const STANDARD = '\\core\\output\\icon_system_standard';
/**
* @const FONTAWESOME Default icon system.
*/
const FONTAWESOME = '\\core\\output\\icon_system_fontawesome';

/**
* @var \core\output\icon_system $instance The cached default instance
*/
private static $instance = null;

/**
* @var array $map A cached mapping of moodle icons to other icons
*/
private $map = null;

/**
* Constructor
*/
private function __construct() {
}

/**
* Factory method
*
* @param $type Either a specific type, or null to get the default type.
* @return \core\output\icon_system
*/
public final static function instance($type = null) {
global $PAGE;

Expand Down
2 changes: 2 additions & 0 deletions lib/classes/output/icon_system_font.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

namespace core\output;

defined('MOODLE_INTERNAL') || die();

/**
* Class allowing different systems for mapping and rendering icons.
*
Expand Down
23 changes: 14 additions & 9 deletions lib/classes/output/icon_system_fontawesome.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
use renderer_base;
use pix_icon;

defined('MOODLE_INTERNAL') || die();

/**
* Class allowing different systems for mapping and rendering icons.
*
Expand All @@ -43,6 +45,11 @@
*/
class icon_system_fontawesome extends icon_system_font {

/**
* @var array $map Cached map of moodle icon names to font awesome icon names.
*/
private $map = [];

public function get_core_icon_map() {
return [
'core:docs' => 'fa-info-circle',
Expand Down Expand Up @@ -89,7 +96,7 @@ public function get_core_icon_map() {
'core:e/delete' => 'fa-minus',
'core:e/delete_table' => 'fa-minus',
'core:e/document_properties' => 'fa-info',
'core:e/emoticons' => 'fa-meh-o',
'core:e/emoticons' => 'fa-smile-o',
'core:e/find_replace' => 'fa-search-plus',
'core:e/forward' => 'fa-arrow-right',
'core:e/fullpage' => 'fa-arrows-alt',
Expand Down Expand Up @@ -241,7 +248,7 @@ public function get_core_icon_map() {
'core:i/completion-manual-enabled' => 'fa-square-o',
'core:i/completion-manual-y' => 'fa-check-square',
'core:i/completion-manual-n' => 'fa-square-o',
'core:i/completion-self' => 'fa-user-o',
'core:i/completion_self' => 'fa-user-o',
'core:i/lock' => 'fa-lock',
'core:i/courseevent' => 'fa-calendar',
'core:i/course' => 'fa-globe',
Expand All @@ -252,10 +259,10 @@ public function get_core_icon_map() {
'core:i/duration' => 'fa-clock-o',
'core:i/edit' => 'fa-pencil',
'core:i/email' => 'fa-envelope',
'core:i/enrolmentsuspended' => 'fa-user-circle',
'core:i/enrolmentsuspended' => 'fa-pause',
'core:i/enrolusers' => 'fa-user-plus',
'core:i/expired' => 'fa-exclamation text-warning',
'core:i/export' => 'fa-level-down',
'core:i/export' => 'fa-download',
'core:i/files' => 'fa-file',
'core:i/filter' => 'fa-filter',
'core:i/flagged' => 'fa-flag',
Expand All @@ -270,7 +277,7 @@ public function get_core_icon_map() {
'core:i/groups' => 'fa-user-circle',
'core:i/groupv' => 'fa-user-circle-o',
'core:i/hide' => 'fa-eye',
'core:i/heirarchylock' => 'fa-lock',
'core:i/hierarchylock' => 'fa-lock',
'core:i/import' => 'fa-level-up',
'core:i/info' => 'fa-info',
'core:i/invalid' => 'fa-exclamation text-danger',
Expand All @@ -289,7 +296,7 @@ public function get_core_icon_map() {
'core:i/moodle_host' => 'fa-graduation-cap',
'core:i/move_2d' => 'fa-arrows',
'core:i/navigationitem' => 'fa-angle-right',
'core:i/ns_red_mark' => 'fa-remove',
'core:i/ne_red_mark' => 'fa-remove',
'core:i/new' => 'fa-plus',
'core:i/news' => 'fa-newspaper-o',
'core:i/nosubcat' => 'fa-plus-square-o',
Expand Down Expand Up @@ -324,7 +331,7 @@ public function get_core_icon_map() {
'core:i/settings' => 'fa-cogs',
'core:i/show' => 'fa-eye-slash',
'core:i/siteevent' => 'fa-share-alt',
'core:i/starrating' => 'fa-star',
'core:i/star-rating' => 'fa-star',
'core:i/stats' => 'fa-line-chart',
'core:i/switch' => 'fa-exchange',
'core:i/switchrole' => 'fa-user-secret',
Expand Down Expand Up @@ -418,8 +425,6 @@ public function get_core_icon_map() {
];
}

private $map = [];

/**
* Overridable function to get a mapping of all icons.
* Default is to do no mapping.
Expand Down
2 changes: 2 additions & 0 deletions lib/classes/output/icon_system_standard.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
use renderer_base;
use pix_icon;

defined('MOODLE_INTERNAL') || die();

/**
* Standard icon rendering. No mapping - img tags used.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,6 @@ EditorPluginButtons.prototype = {
button.one(CSS.MENUEXPAND).append(iconhtml);
});
});


// Append it to the group.
group.append(button);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,6 @@ EditorPluginButtons.prototype = {
button.one(CSS.MENUEXPAND).append(iconhtml);
});
});


// Append it to the group.
group.append(button);
Expand Down
1 change: 0 additions & 1 deletion lib/editor/atto/yui/src/editor/js/editor-plugin-buttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ EditorPluginButtons.prototype = {
button.one(CSS.MENUEXPAND).append(iconhtml);
});
});


// Append it to the group.
group.append(button);
Expand Down
3 changes: 1 addition & 2 deletions lib/externallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public static function call_external_function($function, $args, $ajaxonly=false)
throw new moodle_exception('servicenotavailable', 'webservice');
}

// Do not allow access to write or delete webservices as a public user.
if ($externalfunctioninfo->loginrequired) {
if (defined('NO_MOODLE_COOKIES') && NO_MOODLE_COOKIES && !PHPUNIT_TEST) {
throw new moodle_exception('servicenotavailable', 'webservice');
Expand All @@ -215,8 +216,6 @@ public static function call_external_function($function, $args, $ajaxonly=false)
require_sesskey();
}
}
// Do not allow access to write or delete webservices as a public user.

// Validate params, this also sorts the params properly, we need the correct order in the next part.
$callable = array($externalfunctioninfo->classname, 'validate_parameters');
$params = call_user_func($callable,
Expand Down
1 change: 0 additions & 1 deletion lib/outputlib.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ function theme_get_locked_theme_for_device($device) {
return $CFG->config_php_settings[$themeconfigname];
}


/**
* This class represents the configuration variables of a Moodle theme.
*
Expand Down
9 changes: 3 additions & 6 deletions lib/outputrenderers.php
Original file line number Diff line number Diff line change
Expand Up @@ -2048,12 +2048,10 @@ public function image_icon($pix, $alt, $component='moodle', array $attributes =
/**
* Renders a pix_icon widget and returns the HTML to display it.
*
* @param pix_icon $icon
* @param image_icon $icon
* @return string HTML fragment
*/
protected function render_image_icon(image_icon $icon) {
global $PAGE;

$system = \core\output\icon_system::instance(\core\output\icon_system::STANDARD);
return $system->render_pix_icon($this, $icon);
}
Expand Down Expand Up @@ -2082,8 +2080,6 @@ public function pix_icon($pix, $alt, $component='moodle', array $attributes = nu
* @return string HTML fragment
*/
protected function render_pix_icon(pix_icon $icon) {
global $PAGE;

$system = \core\output\icon_system::instance();
return $system->render_pix_icon($this, $icon);
}
Expand All @@ -2095,7 +2091,8 @@ protected function render_pix_icon(pix_icon $icon) {
* @return string HTML fragment
*/
protected function render_pix_emoticon(pix_emoticon $emoticon) {
return $this->render_pix_icon($emoticon);
$system = \core\output\icon_system::instance(\core\output\icon_system::STANDARD);
return $system->render_pix_icon($this, $emoticon);
}

/**
Expand Down
Loading

0 comments on commit b9b409c

Please sign in to comment.