Skip to content

Allow General Section to be Numbered #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

Allow General Section to be Numbered #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f6dd322
Address issue #22 raised in original repository for this plugin.
Jul 7, 2017
e6be39b
Minor corrections following code checker run.
Jul 7, 2017
4239b96
Added section number bullet point before course page link and minor c…
Jul 7, 2017
f32eb3a
Correction to ensure correct section is highlighted when in a section.
Jul 7, 2017
61d88b7
Minor fix to ensure course page home link is highlighted when on the …
Jul 7, 2017
3132427
Ensure general section highlighted correctly without affecting course…
Jul 7, 2017
17ef39a
Minor updates to formatting and consistent look for each section when…
Jul 10, 2017
b6f00cf
Merge remote-tracking branch 'upstream/master'
Nov 20, 2017
527de25
Updates following original pull request. Made instance level setting…
Nov 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 82 additions & 6 deletions block_course_contents.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,83 @@ public function get_content() {
$title = $format->get_section_name($section);
}

// Check if we want to display a course link. Checked forced status from global config first,
// then check block instance settings.
if ($globalconfig->display_course_link === 'forced_off') {
$displaycourselink = false;

} else if ($globalconfig->display_course_link === 'forced_on') {
$displaycourselink = true;

} else if (empty($this->config) or !isset($this->config->display_course_link)) {
// Instance not configured, use the globally defined default value.
if ($globalconfig->display_course_link === 'optional_on') {
$displaycourselink = true;
} else {
$displaycourselink = false;
}
} else if (!empty($this->config->display_course_link)) {
$displaycourselink = true;

} else {
$displaycourselink = false;

}

if (($i == 0) && ($displaycourselink)) {
$sectionclass = 'section-item';

if ((!isset($selected)) && (empty($selected)) ) {
$sectionclass .= ' current ' . $selected;
}
$text .= html_writer::start_tag('li', array('class' => $sectionclass));

$text .= html_writer::span('>', 'section-number');
if (!empty($this->config->display_course_link_text)) {
$anchortext = $this->config->display_course_link_text;
} else if (!empty($globalconfig->display_course_link_text)) {
$anchortext = $globalconfig->display_course_link_text;
} else {
$anchortext = $course->shortname;
}

if ((!isset($selected)) && (empty($selected)) ) {
$text .= '  ' . $anchortext;
} else {
$text .= '  ' . html_writer::link(course_get_url($course), $anchortext);
}

$text .= html_writer::end_tag('li');
}

$odd = $r % 2;
if ($format->is_section_current($section)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to remove the existing feature of highlighting the current section?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From our testing it seems to highlight fine?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just re-checked and I believe there was some misunderstanding. Your has changed the meaning of the class "current" to match the section the user currently clicked to. But originally this was a section highlighted as the "current" one by the teacher of the course.

Anyway, I realized your intention and I agree that highlighting the currently selected section should be more prominent than highlighting the current one. I am adding an extra change that introduces a new CSS class "selected" which will use the original "current" styling. The "current" section will be still highlighted but not that much.

This is an example of the result of my fix: in the following example, the user is viewing the section "Topic 4" and the teacher has marked the "Topic 2" as the current one. Your patch did not highlight the "Topic 2" at all, my new version will show it in bold:

image

$text .= html_writer::start_tag('li', array('class' => 'section-item current r'.$odd));
if (isset($selected) && $i == $selected) {
$text .= html_writer::start_tag('li', array('class' => 'section-item current r.$odd'));
} else {
$text .= html_writer::start_tag('li', array('class' => 'section-item r'.$odd));
}

if ($i == 0) {
// Never enumerate the section number 0.
// Check if we want to enumerate section 0. Checked forced status from global config first,
// then check block instance settings.
if ($globalconfig->enumerate_section_0 === 'forced_off') {
$enumeratesection0 = false;
} else if ($globalconfig->enumerate_section_0 === 'forced_on') {
$enumeratesection0 = true;
} else if (empty($this->config) or !isset($this->config->enumerate_section_0 )) {
// Instance not configured, use the globally defined default value.
if ($globalconfig->enumerate_section_0 === 'optional_on') {
$enumeratesection0 = true;
} else {
$enumeratesection0 = false;
}
} else if (!empty($this->config->enumerate_section_0 )) {
$enumeratesection0 = true;
} else {
$enumeratesection0 = false;
}

if ( ($i == 0) && ($enumeratesection0 == false) ) {
// Never enumerate the section number 0 unless option has been set.
$enumerate = false;

} else if ($globalconfig->enumerate === 'forced_off') {
Expand All @@ -196,8 +264,16 @@ public function get_content() {
$enumerate = false;
}

$sectionnumber = $i;

// If enumerating and showing section 0, then increment section number.
if ( ($enumerate == true) && ($enumeratesection0 == true)) {
$sectionnumber++;
}

if ($enumerate) {
$title = html_writer::span($i, 'section-number').' '.html_writer::span($title, 'section-title');
$title = html_writer::span($sectionnumber, 'section-number'). ' '
. html_writer::span(' ' . $title, 'section-title');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of the   here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a little extra spacing between the number and section title.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am removing it and adding an extra space properly via CSS.


} else {
$title = html_writer::span($title, 'section-title not-enumerated');
Expand Down Expand Up @@ -258,4 +334,4 @@ private function node_plain_text($node) {
}
return $t;
}
}
}
60 changes: 60 additions & 0 deletions edit_form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,65 @@ protected function specific_definition($mform) {
}

$mform->setType('config_autotitle', PARAM_BOOL);

// Enumerate section 0.
if ($config->enumerate_section_0 === 'forced_off') {
$mform->addElement('static', 'config_enumerate_section_0_info', get_string('config_enumerate_section_0',
'block_course_contents'), get_string('config_enumerate_section_0_forced_off', 'block_course_contents'));
$mform->addHelpButton('config_enumerate_section_0_info', 'config_enumerate_section_0', 'block_course_contents');
$mform->addElement('hidden', 'config_enumerate_section_0');

} else if ($config->enumerate_section_0 === 'forced_on') {
$mform->addElement('static', 'config_enumerate_section_0_info', get_string('config_enumerate_section_0',
'block_course_contents'), get_string('config_enumerate_section_0_forced_on', 'block_course_contents'));
$mform->addHelpButton('config_enumerate_section_0_info', 'config_enumerate_section_0', 'block_course_contents');
$mform->addElement('hidden', 'config_enumerate_section_0');

} else {
$mform->addElement('advcheckbox', 'config_enumerate_section_0', get_string('config_enumerate_section_0',
'block_course_contents'), get_string('config_enumerate_section_0_desc', 'block_course_contents'));
$mform->addHelpButton('config_enumerate_section_0', 'config_enumerate_section_0', 'block_course_contents');

if ($config->enumerate_section_0 === 'optional_on') {
$mform->setDefault('config_enumerate_section_0', 1);

} else {
$mform->setDefault('config_enumerate_section_0', 0);
}
}
$mform->setType('config_enumerate_section_0', PARAM_BOOL);

// Display course page link.
if ($config->display_course_link === 'forced_off') {
$mform->addElement('static', 'config_display_course_link_info', get_string('config_display_course_link',
'block_course_contents'), get_string('config_display_course_link_forced_off', 'block_course_contents'));
$mform->addHelpButton('config_display_course_link_info', 'config_display_course_link', 'block_course_contents');
$mform->addElement('hidden', 'config_display_course_link');

} else if ($config->display_course_link === 'forced_on') {
$mform->addElement('static', 'config_display_course_link_info', get_string('config_display_course_link',
'block_course_contents'), get_string('config_display_course_link_forced_on', 'block_course_contents'));
$mform->addHelpButton('config_display_course_link_info', 'config_display_course_link', 'block_course_contents');
$mform->addElement('hidden', 'config_display_course_link');

} else {
$mform->addElement('advcheckbox', 'config_display_course_link', get_string('config_display_course_link',
'block_course_contents'), get_string('config_display_course_link_desc', 'block_course_contents'));
$mform->addHelpButton('config_display_course_link', 'config_display_course_link', 'block_course_contents');

if ($config->display_course_link === 'optional_on') {
$mform->setDefault('config_display_course_link', 1);

} else {
$mform->setDefault('config_display_course_link', 0);
}
}
$mform->setType('config_display_course_link', PARAM_BOOL);

$mform->addElement('text', 'config_display_course_link_text',
get_string('config_display_course_link_text_desc', 'block_course_contents'));
$mform->setDefault('config_display_course_link_text', '');
$mform->setType('config_display_course_link_text', PARAM_RAW);

}
}
23 changes: 22 additions & 1 deletion lang/en/block_course_contents.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,25 @@
$string['config_enumerate_optional_on'] = 'Optional, enabled by default';
$string['course_contents:addinstance'] = 'Add a new course contents block';
$string['notusingsections'] = 'This course format does not use sections.';
$string['pluginname'] = 'Course contents';
$string['pluginname'] = 'Course contents';

$string['config_enumerate_section_0'] = 'Enumerate section 0 / General';
$string['config_enumerate_section_0_desc'] = 'Start enumerating from section 0 / General (if displaying one section per page)';
$string['config_enumerate_section_0_forced_off'] = 'Disabled in all blocks';
$string['config_enumerate_section_0_forced_on'] = 'Enabled in all blocks';
$string['config_enumerate_section_0_optional_off'] = 'Optional, disabled by default';
$string['config_enumerate_section_0_optional_on'] = 'Optional, enabled by default';
$string['config_enumerate_section_0_help'] = 'Setting this option will enumerate from the first section, usually known as section 0 / General section.' .
' This can be set to apply to all blocks and also overriden at the block instance level.';

$string['config_display_course_link'] = 'Display course page link';
$string['config_display_course_link_desc'] = 'Display course home page link at the top of all sections.';
$string['config_display_course_link_forced_off'] = 'Disabled in all blocks';
$string['config_display_course_link_forced_on'] = 'Enabled in all blocks';
$string['config_display_course_link_optional_off'] = 'Optional, disabled by default';
$string['config_display_course_link_optional_on'] = 'Optional, enabled by default';
$string['config_display_course_link_help'] = 'This will display a link to the course home page above all course section links.' .
' This can be set to apply to all blocks and also overriden at the block instance level.';

$string['config_display_course_link_text'] = 'Display Course page link anchor text';
$string['config_display_course_link_text_desc'] = 'Display course home page link anchor text (if blank, course shortcode will be used).';
37 changes: 37 additions & 0 deletions settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,43 @@
$enumerate
));

// Enumerate section 0.
$enumeratesection0 = [
'forced_on' => get_string('config_enumerate_section_0_forced_on', 'block_course_contents'),
'optional_on' => get_string('config_enumerate_section_0_optional_on', 'block_course_contents'),
'optional_off' => get_string('config_enumerate_section_0_optional_off', 'block_course_contents'),
'forced_off' => get_string('config_enumerate_section_0_forced_off', 'block_course_contents'),
];

$settings->add(new admin_setting_configselect(
'block_course_contents/enumerate_section_0',
get_string('config_enumerate_section_0', 'block_course_contents'),
get_string('config_enumerate_section_0_desc', 'block_course_contents'),
'optional_off',
$enumeratesection0
));

// Display course page link.
$displaycourselink = [
'forced_on' => get_string('config_display_course_link_forced_on', 'block_course_contents'),
'optional_on' => get_string('config_display_course_link_optional_on', 'block_course_contents'),
'optional_off' => get_string('config_display_course_link_optional_off', 'block_course_contents'),
'forced_off' => get_string('config_display_course_link_forced_off', 'block_course_contents'),
];

$settings->add(new admin_setting_configselect(
'block_course_contents/display_course_link',
get_string('config_display_course_link', 'block_course_contents'),
get_string('config_display_course_link_desc', 'block_course_contents'),
'optional_off',
$displaycourselink
));

// Course page link custom text.
$settings->add(new admin_setting_configtext('block_course_contents/display_course_link_text',
new lang_string('config_display_course_link_text', 'block_course_contents'),
new lang_string('config_display_course_link_text_desc', 'block_course_contents'), ''));

$autotitle = [
'forced_on' => get_string('config_autotitle_forced_on', 'block_course_contents'),
'optional_on' => get_string('config_autotitle_optional_on', 'block_course_contents'),
Expand Down