Skip to content
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.

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

Inherit hidden 2 #107

Closed
wants to merge 3 commits into from
Closed

Inherit hidden 2 #107

wants to merge 3 commits into from

Conversation

james-cnz
Copy link
Contributor

This is another attempt to close #83. It's smaller than #98, and it seems to work, including for activity navigation, which was a problem before. It probably needs more testing though. I'm also not sure if performance will be OK on large courses.

@james-cnz
Copy link
Contributor Author

I tried a bit of performance testing on this. I did 20 page loads in my Moodle sandbox of a course with 50 sections, with and without the change, with performance info turned on. With the change averaged only about 1% slower. I don't know how accurate this is, but it seems promising.

@davidherney
Copy link
Owner

Thanks @james-cnz ... I am going to test it in order to integrate in the new release.

@davidherney
Copy link
Owner

Hi @james-cnz ...

I did merge your code with some little changes in the next branch:
https://github.com/davidherney/moodle-format_onetopic/tree/MOODLE_39_inherit_hidden

Please, help me to test before upgrade the new official version.

Saludos

@james-cnz
Copy link
Contributor Author

I think there is an error. (I don't think it's my change though, I think it's related to #97.)
If a first child tab is hidden, no children are shown when the parent tab is selected. It looks like this is due to renderer.php line:
if ($displaysection == $section - 1) {
this would be better as:
if ($displaysection == $parentsection->section) {
But this is still not quite right, because there can be an error when section 0 is displayed before the tabs and section 1 is set to be a child tab. Also this needs to be changed:
} while ($parentformatoptions['level'] == 1 && $prevsectionindex >= 0);
to, e.g.:
} while ($parentformatoptions['level'] == 1 && $prevsectionindex >= $firstsection);
and add:
$firstsection = ($course->realcoursedisplay == COURSE_DISPLAY_MULTIPAGE) ? 1 : 0;

@davidherney
Copy link
Owner

Not really because the first tab always is a parent tab, the level parameter is omitted in this case with the condition:
$parenttab == null
in line:
if ($formatoptions['level'] == 0 || $parenttab == null) {

I was testing both cases and not found error. Please, test it in a course.

Gracias

Saludos

@james-cnz
Copy link
Contributor Author

I did test, but I guess I didn't explain very well.
Steps are:
1/ Create new Onetopic course with 3 topics.
2/ Set hidden sections completely invisible.
3/ Set Topic 2 to child and hidden.
4/ Set Topic 3 to child.
5/ Open Topic 1.
6/ Switch role to student.
Topic 3 should be visible, but isn't.
7/ change renderer.php line 369 from:
if ($displaysection == $section - 1) {
to:
if ($displaysection == $parentsection->section) {
Problem is fixed (maybe need to purge caches to see).
Except now there is a new problem instead:
8/ Set Section 0 before the tabs.
9/ Set topic 1 to child.
Topics 2 and 3 should be visible, but aren't.
10/ Change renderer.php line 359 from:
} while ($parentformatoptions['level'] == 1 && $prevsectionindex >= 0);
to:
} while ($parentformatoptions['level'] == 1 && $prevsectionindex >= $firstsection);
and add before:
$firstsection = ($course->realcoursedisplay == COURSE_DISPLAY_MULTIPAGE) ? 1 : 0;
New problem is fixed (again, maybe need to purge caches to see).

@davidherney
Copy link
Owner

ah... ok... you have all the reason. The step by step example is useful to test. Thanks for it.

I fixed it now as you suggest.

Saludos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Child tabs should inherit "hidden from students" from parent tab
2 participants