Allow General Section to be Numbered

[jerryhops]

This block is very useful when using Grid format (or others) set to display "one section per page".

Two features would really make a difference in this context:

1. Add an option to "enumerate section 0"
2. Add an option "add link to course page" using shortcode as the anchor text

With these options when we "snap to grid" the general section in Grid it could be enumerated.
With the course page option enabled there would be a convenient link back to the course page.

[jerryhops]

The pull request linked to this issue allows the Course Contents block to work with grid as shown in the screen recording below:

https://1drv.ms/v/s!Aqw_f9o-xclcg743-Kc99W6D5YuuJA

In that test course the "general" section has been renamed to "introduction" and snapped to the grid (a feature of that course format). That course has also been set to display one section per page.

[mudrd8mz]

Hmm... The side effect of these new settings is that sections like "Topic X" are enumerated as "Topic X+1" which is something I wanted to avoid. Also by default, visiting the General section (zero) in the standard topics format actually displays the whole course page.

But I understand these two options fit better to your Grid format. I'll play with it a little more and will let you know. The submitted pull request helps and gives me a good start.

[jerryhops]

Hi David,

Yes we were aware of those issues which is why they were added as settings / options rather than being enabled by default.

Anyone upgrading the plugin would see no change but those that wanted to use it in a different way could enable those settings.

That is not ideal and could no doubt be improved, perhaps by checking the course settings:

if (course_format == "grid" && course_layout == "show one section per page")

else if (course_layout == "show one section per page")

[mudrd8mz]

if (course_format == "grid" && course_layout == "show one section per page")

No please. Just making it properly configurable (defaults to be set as global settings, each block instance able to have its own setting) will do it.

[jerryhops]

Ok, currently the version in the pull request already has these settings at admin level.

If these were also added to the instance settings would you be happy with that or do you think the settings need more work too?

[mudrd8mz]

I submitted a review of the pull request a short while ago. Having those settings on global level only is not good. Global settings should serve as default values for each instance's own properties.

[5882wolverine]

Hi David,

I'm not sure if you've seen it but I add some more commits to the same pull request about a month ago? This should address the points raised from the previous review. Or would it be better to raise a new pull request?

Thanks,

M

[mudrd8mz]

D'oh sorry, it disappeared from my radar somehow. I'll get to it this week, thanks!

[5882wolverine]

No problem. Just saw your message. Not sure if you'd reviewed it again?

Thanks!