Skip to content

Commit

Permalink
MDL-69649 backup: Fix missing labels
Browse files Browse the repository at this point in the history 
- The backup details page uses a table to show a sumary of the backup
  content. Used role attribute to denote the tabular format of the
  summary.
- The backup details page displays activity name next to each activity
  icon. Therefore the icons are only decorative and do not need to have
  any title or even alt text.
- Form labels should be associated with form controls.  A div element is
  not a form control.
- The from attribute of the form labels should be equal to the id
  attribute of an element. Therefore, we first create a label and an
  input elements and associate them to each other, and then pass them to
  backup_detail_pair() when a label is needed.
  • Loading branch information
@rezaies
rezaies committed Sep 28, 2020
1 parent a348c5f commit d54b344
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 33 deletions.
106 changes: 73 additions & 33 deletions backup/util/ui/renderer.php
View file
Expand Up @@ -126,8 +126,9 @@ public function backup_details($details, $nextstageurl) {

$html = html_writer::start_tag('div', array('class' => 'backup-restore'));

$html .= html_writer::start_tag('div', array('class' => 'backup-section'));
$html .= $this->output->heading(get_string('backupdetails', 'backup'), 2, array('class' => 'header'));
$html .= html_writer::start_tag('div', ['class' => 'backup-section',
'role' => 'table', 'aria-labelledby' => 'backupdetailsheader']);
$html .= $this->output->heading(get_string('backupdetails', 'backup'), 2, 'header', 'backupdetailsheader');
$html .= $this->backup_detail_pair(get_string('backuptype', 'backup'), get_string('backuptype'.$details->type, 'backup'));
$html .= $this->backup_detail_pair(get_string('backupformat', 'backup'), get_string('backupformat'.$details->format, 'backup'));
$html .= $this->backup_detail_pair(get_string('backupmode', 'backup'), get_string('backupmode'.$details->mode, 'backup'));
Expand All @@ -153,8 +154,9 @@ public function backup_details($details, $nextstageurl) {

$html .= html_writer::end_tag('div');

$html .= html_writer::start_tag('div', array('class' => 'backup-section settings-section'));
$html .= $this->output->heading(get_string('backupsettings', 'backup'), 2, array('class' => 'header'));
$html .= html_writer::start_tag('div', ['class' => 'backup-section settings-section',
'role' => 'table', 'aria-labelledby' => 'backupsettingsheader']);
$html .= $this->output->heading(get_string('backupsettings', 'backup'), 2, 'header', 'backupsettingsheader');
foreach ($details->root_settings as $label => $value) {
if ($label == 'filename' or $label == 'user_files') {
continue;
Expand All @@ -164,8 +166,9 @@ public function backup_details($details, $nextstageurl) {
$html .= html_writer::end_tag('div');

if ($details->type === 'course') {
$html .= html_writer::start_tag('div', array('class' => 'backup-section'));
$html .= $this->output->heading(get_string('backupcoursedetails', 'backup'), 2, array('class' => 'header'));
$html .= html_writer::start_tag('div', ['class' => 'backup-section',
'role' => 'table', 'aria-labelledby' => 'backupcoursedetailsheader']);
$html .= $this->output->heading(get_string('backupcoursedetails', 'backup'), 2, 'header', 'backupcoursedetailsheader');
$html .= $this->backup_detail_pair(get_string('coursetitle', 'backup'), $details->course->title);
$html .= $this->backup_detail_pair(get_string('courseid', 'backup'), $details->course->courseid);

Expand Down Expand Up @@ -200,7 +203,7 @@ public function backup_details($details, $nextstageurl) {
$table->data = array();
}
$name = get_string('pluginname', $activity->modulename);
$icon = new image_icon('icon', $name, $activity->modulename, array('class' => 'iconlarge icon-pre'));
$icon = new image_icon('icon', '', $activity->modulename, ['class' => 'iconlarge icon-pre']);
$table->data[] = array(
$this->output->render($icon).$name,
$activity->title,
Expand Down Expand Up @@ -424,13 +427,25 @@ public function import_course_selector(moodle_url $nextstageurl, import_course_s
protected function backup_detail_pair($label, $value) {
static $count = 0;
$count ++;
$html = html_writer::start_tag('div', array('class' => 'detail-pair'));
$html .= html_writer::tag('label', $label, array('class' => 'detail-pair-label', 'for' => 'detail-pair-value-'.$count));
$html .= html_writer::tag('div', $value, array('class' => 'detail-pair-value pl-2', 'name' => 'detail-pair-value-'.$count));
$html = html_writer::start_tag('div', ['class' => 'detail-pair', 'role' => 'row']);
$html .= html_writer::tag('div', $label, ['class' => 'detail-pair-label mb-2', 'role' => 'cell']);
$html .= html_writer::tag('div', $value, ['class' => 'detail-pair-value pl-2', 'role' => 'cell']);
$html .= html_writer::end_tag('div');
return $html;
}

/**
* Creates a unique id string by appending an incremental number to the prefix.
*
* @param string $prefix To be used as the left part of the id string.
* @return string
*/
protected function make_unique_id(string $prefix): string {
static $count = 0;

return $prefix . '-' . $count++;
}

/**
* Created a detailed pairing with an input
*
Expand All @@ -448,9 +463,11 @@ protected function backup_detail_input($label, $type, $name, $value, array $attr
} else {
$description = '';
}
$id = $this->make_unique_id('detail-pair-value');
return $this->backup_detail_pair(
$label,
html_writer::empty_tag('input', $attributes + array('name' => $name, 'type' => $type, 'value' => $value)) . $description
html_writer::label($label, $id),
html_writer::empty_tag('input', $attributes + ['id' => $id, 'name' => $name, 'type' => $type, 'value' => $value]) .
$description
);
}

Expand Down Expand Up @@ -718,8 +735,6 @@ public function render_backup_files_viewer(backup_files_viewer $viewer) {
* @return string
*/
public function render_restore_course_search(restore_course_search $component) {
$url = $component->get_url();

$output = html_writer::start_tag('div', array('class' => 'restore-course-search form-inline mb-1'));
$output .= html_writer::start_tag('div', array('class' => 'rcs-results table-sm w-75'));

Expand All @@ -733,11 +748,18 @@ public function render_restore_course_search(restore_course_search $component) {
if (!$course->visible) {
$row->attributes['class'] .= ' dimmed';
}
$row->cells = array(
html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'targetid', 'value' => $course->id)),
format_string($course->shortname, true, array('context' => context_course::instance($course->id))),
format_string($course->fullname, true, array('context' => context_course::instance($course->id)))
);
$id = $this->make_unique_id('restore-course');
$row->cells = [
html_writer::empty_tag('input', ['type' => 'radio', 'name' => 'targetid', 'value' => $course->id,
'id' => $id]),
html_writer::label(
format_string($course->shortname, true, ['context' => context_course::instance($course->id)]),
$id,
true,
['class' => 'd-block']
),
format_string($course->fullname, true, ['context' => context_course::instance($course->id)])
];
$table->data[] = $row;
}
if ($component->has_more_results()) {
Expand All @@ -764,6 +786,8 @@ public function render_restore_course_search(restore_course_search $component) {
'type' => 'text',
'name' => restore_course_search::$VAR_SEARCH,
'value' => $component->get_search(),
'aria-label' => get_string('searchcourses'),
'placeholder' => get_string('searchcourses'),
'class' => 'form-control'
);
$output .= html_writer::empty_tag('input', $attrs);
Expand All @@ -787,8 +811,6 @@ public function render_restore_course_search(restore_course_search $component) {
* @return string
*/
public function render_import_course_search(import_course_search $component) {
$url = $component->get_url();

$output = html_writer::start_tag('div', array('class' => 'import-course-search'));
if ($component->get_count() === 0) {
$output .= $this->output->notification(get_string('nomatchingcourses', 'backup'));
Expand All @@ -798,6 +820,8 @@ public function render_import_course_search(import_course_search $component) {
'type' => 'text',
'name' => restore_course_search::$VAR_SEARCH,
'value' => $component->get_search(),
'aria-label' => get_string('searchcourses'),
'placeholder' => get_string('searchcourses'),
'class' => 'form-control'
);
$output .= html_writer::empty_tag('input', $attrs);
Expand Down Expand Up @@ -833,11 +857,18 @@ public function render_import_course_search(import_course_search $component) {
if (!$course->visible) {
$row->attributes['class'] .= ' dimmed';
}
$row->cells = array(
html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'importid', 'value' => $course->id)),
format_string($course->shortname, true, array('context' => context_course::instance($course->id))),
format_string($course->fullname, true, array('context' => context_course::instance($course->id)))
);
$id = $this->make_unique_id('import-course');
$row->cells = [
html_writer::empty_tag('input', ['type' => 'radio', 'name' => 'importid', 'value' => $course->id,
'id' => $id]),
html_writer::label(
format_string($course->shortname, true, ['context' => context_course::instance($course->id)]),
$id,
true,
['class' => 'd-block']
),
format_string($course->fullname, true, ['context' => context_course::instance($course->id)])
];
$table->data[] = $row;
}
if ($component->has_more_results()) {
Expand All @@ -856,6 +887,8 @@ public function render_import_course_search(import_course_search $component) {
'type' => 'text',
'name' => restore_course_search::$VAR_SEARCH,
'value' => $component->get_search(),
'aria-label' => get_string('searchcourses'),
'placeholder' => get_string('searchcourses'),
'class' => 'form-control');
$output .= html_writer::empty_tag('input', $attrs);
$attrs = array(
Expand All @@ -878,8 +911,6 @@ public function render_import_course_search(import_course_search $component) {
* @return string
*/
public function render_restore_category_search(restore_category_search $component) {
$url = $component->get_url();

$output = html_writer::start_tag('div', array('class' => 'restore-course-search form-inline mb-1'));
$output .= html_writer::start_tag('div', array('class' => 'rcs-results table-sm w-75'));

Expand All @@ -895,12 +926,19 @@ public function render_restore_category_search(restore_category_search $componen
$row->attributes['class'] .= ' dimmed';
}
$context = context_coursecat::instance($category->id);
$row->cells = array(
html_writer::empty_tag('input', array('type' => 'radio', 'name' => 'targetid', 'value' => $category->id)),
format_string($category->name, true, array('context' => context_coursecat::instance($category->id))),
$id = $this->make_unique_id('restore-category');
$row->cells = [
html_writer::empty_tag('input', ['type' => 'radio', 'name' => 'targetid', 'value' => $category->id,
'id' => $id]),
html_writer::label(
format_string($category->name, true, ['context' => context_coursecat::instance($category->id)]),
$id,
true,
['class' => 'd-block']
),
format_text(file_rewrite_pluginfile_urls($category->description, 'pluginfile.php', $context->id,
'coursecat', 'description', null), $category->descriptionformat, array('overflowdiv' => true))
);
'coursecat', 'description', null), $category->descriptionformat, ['overflowdiv' => true])
];
$table->data[] = $row;
}
if ($component->has_more_results()) {
Expand All @@ -927,6 +965,8 @@ public function render_restore_category_search(restore_category_search $componen
'type' => 'text',
'name' => restore_category_search::$VAR_SEARCH,
'value' => $component->get_search(),
'aria-label' => get_string('searchcoursecategories'),
'placeholder' => get_string('searchcoursecategories'),
'class' => 'form-control'
);
$output .= html_writer::empty_tag('input', $attrs);
Expand Down
1 change: 1 addition & 0 deletions lang/en/moodle.php
View file
Expand Up @@ -1810,6 +1810,7 @@
$string['searchactivities'] = 'Search for activities by name or description';
$string['searchbyemail'] = 'Search by email address';
$string['searchbyusername'] = 'Search by username';
$string['searchcoursecategories'] = 'Search categories';
$string['searchcourses'] = 'Search courses';
$string['searchoptions'] = 'Search options';
$string['searchresults'] = 'Search results';
Expand Down

0 comments on commit d54b344

Please sign in to comment.