-
Improvement
-
Resolution: Fixed
-
Minor
-
2.5.1, 2.9
-
MOODLE_25_STABLE, MOODLE_29_STABLE
-
MOODLE_29_STABLE
-
MDL-41608-master -
-
5
Grrrr took me 5 minutes to track this down, theme_bootstrapbase_core_renderer::notification doesn't apply any custom classes. It looks for just 4 classes and disregards the rest.
Also it expects that $classes is a string which is not the case, it is valid to pass and array to classes as well.
/*
|
* This renders a notification message.
|
* Uses bootstrap compatible html.
|
*/
|
public function notification($message, $classes = 'notifyproblem') {
|
$message = clean_text($message);
|
$type = '';
|
|
if ($classes == 'notifyproblem') {
|
$type = 'alert alert-error';
|
}
|
if ($classes == 'notifysuccess') {
|
$type = 'alert alert-success';
|
}
|
if ($classes == 'notifymessage') {
|
$type = 'alert alert-info';
|
}
|
if ($classes == 'redirectmessage') {
|
$type = 'alert alert-block alert-info';
|
}
|
return "<div class=\"$type\">$message</div>";
|
}
|
Edit 09/Aug/2014
Summarizing this issue:
- Sam initially discovered the issue on 05/Sep/13, wherein theme_bootstrapbase_core_renderer::notification doesn't apply any custom classes.
- Discussion took place and David sketched out a new notification API where rather than calling notification with one or more classes, notifications should be emitted by using one of alert_info, alert_success, alert_warning, alert_danger depending on severity level.
- Jason reviewed David's changes.
- Jetha discovered Sam's initial issue in 2014 when trying to add support for detailed notifications (e.g. CSV import stack traces), and a discussion at HQ took place - the conclusion of which is that while deprecating notification and its use in core is the right thing to do, the surface area to amend is too large to do in one issue.
This issue contained three things:
- Fixing the behaviour Sam initially identified, wherein theme_bootstrapbase_core_renderer::notification doesn't apply any custom classes
- This is achieved by removing theme_bootstrapbase_core_renderer::notification entirely such that core_renderer::notification can handle things with the help of a new array in theme_bootstrapbase_core_renderer containing mappings between Moodle and Bootstrap CSS classes.
- Integrating David's new notification API, but not yet deprecating notification (which will be done in a separate issue, probably after the Gradebook 2014 push).
- Adding support for an optional details parameter on both old and new notification APIs for more detailed messages.
Edit 18/Mar/2015
This issue is now simply about:
- adding support for adding additional classes to a notification beyond simply notifyxxx
- creating renderables
- rendering notifications with Mustache, see
MDL-49152
- Discovered while testing
-
MDL-41647 Web services uses non-existant CSS class "success" instead of "notifysuccess"
-
- Closed
-
-
MDL-41644 Backup uses a nonexistent CSS class "notifywarning" twice
-
- Closed
-
-
MDL-41646 Wiki has odd wiki_info CSS to underline some error messages
-
- Closed
-
-
MDL-41648 Assignment and Grade notifications use 'error' class rather than 'notifyproblem'
-
- Closed
-
-
MDL-41650 Tags use unsemantic .red and .green classes for notification rather than .notifysuccess and .notifyproblem
-
- Closed
-
- is blocked by
-
MDL-49152 Templates for renderers
-
- Closed
-
- is duplicated by
-
MDL-46631 Bootstrap notification logic inconsistent with core
-
- Closed
-
- Testing discovered
-
MDL-49800 Do not allow templates in sub folders.
-
- Closed
-