moodle-theme_essential
moodle-theme_essential copied to clipboard
gjb2048
Icon replacement seems broken
Hi Gareth,
I just realized that there is an instance where pix replacement does not work (M3.2; latest version of theme_essential). Have a look at course/view.php with editing mode switched on:
Replacement of pix used in the dropdown and for the groupmode toogler do not work.
Up until Moodle 3.1 we used the "old" activity editing controls, i.e. the non-menu ones. It seems that now it is not possible to use them anymore, so I am looking at the menu for the first time. Therefore I can't tell whether this is something new or something that existed for some time now.
I will try to find time to figure this out.
Regards Jan
Alright, sorry, this is mostly a Moodle problem, as it initialises all this icons using the pix_icon constructor; e.g. activity editing:
\course_get_cm_edit_actions():
$actions['update'] = new action_menu_link_secondary(
new moodle_url($baseurl, array('update' => $mod->id)),
new pix_icon('t/edit', $str->editsettings, 'moodle', array('class' => 'iconsmall', 'title' => '')),
$str->editsettings,
array('class' => 'editing_update', 'data-action' => 'update')
);
e.g. section editing:
\format_section_renderer_base::section_edit_control_menu():
$al = new action_menu_link_secondary(
new moodle_url($url),
new pix_icon($icon, $name, null, array('class' => "smallicon " . $class, 'alt' => $alt)),
$name,
$attr
);
Will report in Moodle Tracker and then we'll see. Sorry for bothering you!
In theory, yes. But if it isn't, there isn't anything you/we can do within theme_essential.
The original problem is that the current renderer/template combination in Moodle core specifies – in many places, such as here – templates that directly import the core/pix_icon template, instead of invoking the corresponding renderer.
This means that we can override the core/pix_icon template in theme_essential, but we can't do any special rendering that would add variables, e.g. for FA replacements – because theme_essential's renderer is not called.
In my opinion, this issue could be closed, but I leave that decision to you, @gjb2048 :)
Thanks for your time and help!
Hi Jan,
The problem is the AJAX code directly injecting the images rather than calling the pix_icon etc. PHP code and thus not calling the overridden theme method. This I discovered a few years back, so do you have new information where a change in the mustache template would solve it please?
Gareth
Sorry, I wasn't too clear. My point was that just adding/editing/overriding a template would not help. Otherwise I would have proposed changes, because that would be easy :).
The reason is that there are core templates that include core/pix_icon directly, cf. https://github.com/moodle/moodle/blob/57a38cf813bb4cb4a9ed8350d215f0f403767b81/lib/templates/action_menu_item.mustache#L30 (MOODLE_32_STABLE)
... and including a template does not invoke any corresponding renderers, so we are lost.
However, while I was looking for the above link to show you, I found something interesting in master:
https://github.com/moodle/moodle/blob/bf919ddf021cacb6711bd00fa3b3b97019ad450a/lib/templates/action_menu_item.mustache
So apparently M3.3 will be shipping a new(?) {{#pixicon}} helper that may come to our rescue. git blame says that this particular change was also done for https://tracker.moodle.org/browse/MDL-40759 which you posted above.
I've been testing with M3.3 and even though there are replacements, the Navigation block still seems to have the bug. Probably because its not used in Boost. Will have to see when the finished code is in.
The navigation block renderer seems to call render on a pix icon, translating into render_pix_icon, so that would surprise me. I don't understand the settings block renderer, though, it is possible that this does not properly render icons!