Fix for invalid HTML in case of zero section elements.

[mwehr]

Hi,

This is a small fix for ul, div closing tags without corresponding open tags -> invalid HTML in case of zero section elements.

regards, Mario

[gjb2048]

I see. Have you tested the solution when there are sections? As there is the line: unset($sections[$thissection->section]); and thus by the time the changed code is reached, then $sections will always be empty.

[gjb2048]

Hence it failing the PHP unit test where there is no closing 'ul' tag for when there are sections. Have you run the PHP Unit tests?

[gjb2048]

The problem is not the 'ul' closing tag as that logic is already catered for with no sections, but:

        if ((!$this->formatresponsive) && ($this->tcsettings['layoutcolumnorientation'] == 1)) { // Vertical columns.
            echo html_writer::end_tag('div');
        }

not having:

        if ((!$this->formatresponsive) && ($this->tcsettings['layoutcolumnorientation'] == 1)) { // Vertical columns.
            echo html_writer::start_tag('div', array('class' => $this->get_row_class()));
        }

when there are zero sections.

[gjb2048]

Therefore there needs to be a better solution along with a new unit test please Mario.

[mwehr]

Hi Gareth,

Hmm ok it seems to be a bit tricky :-) I think the problem is the closing "div" in the "formatresponsive" part. My solution would be to use an additional condition "$coursenumsections > 0" in the IF statement for the "formatresponsive". The start "div" tag of the responsive part is added only if $coursenumsections > 0 is true so this should fix the problem.

 if ($coursenumsections > 0 && !$this->formatresponsive && ($this->tcsettings['layoutcolumnorientation'] == 1)) { // Vertical columns.
            echo html_writer::end_tag('div');
        }

would do you think ?

[gjb2048]

I think it needs to be simpler.

[gjb2048]

FYI: https://github.com/gjb2048/moodle-format_topcoll/commit/0f35c6ac937db14ed3c37f400854b08383c2cc4a

[mwehr]

Great,... and thx for the credits :-)