search

james-cnz / moodle-format_multitopic

Shows multiple topics per page, with tabbed navigation between pages.
GNU General Public License v3.0
5 stars 7 forks source link

Sharing Cart #13

Closed bradnielsen2981 closed 4 years ago

bradnielsen2981 commented 4 years ago

This isn't necessarily an issue with multi-topic but the Sharing Cart plugin doesn't work with tabbed sections. It creates an error.

james-cnz commented 4 years ago

Hi Brad,

Could you give me some more details on this? I had a quick play with Sharing Cart and haven't run into an error.

bradnielsen2981 commented 4 years ago

Hi James, the error is when you try to copy a section that contains other sections. Do you get an error then? I'm using Moodle 3.5. The error I get is: A required parameter (sectionname) was missing

Sent from Outlookhttp://aka.ms/weboutlook

From: James C notifications@github.com Sent: Saturday, 5 September 2020 8:19 PM To: james-cnz/moodle-format_multitopic moodle-format_multitopic@noreply.github.com Cc: Brad Nielsen brad_nielsen29@hotmail.com; Author author@noreply.github.com Subject: Re: [james-cnz/moodle-format_multitopic] Sharing Cart (#13)

Hi Brad,

Could you give me some more details on this? I had a quick play with Sharing Cart and haven't run into an error.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/james-cnz/moodle-format_multitopic/issues/13#issuecomment-687585224, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJC4HKE4ZMYJAQLMZOCAW5LSEIGBRANCNFSM4QJECX7Q.

james-cnz commented 4 years ago

Hi Brad,

Thanks for this. I didn't notice the option to copy a section, I was just playing with the option to copy activities.

I think I have just made this worse with Multitopic 2.0. It now uses the property aria-labelledby on sections, like the Topics/Weekly formats in Moodle 3.9, rather than the property aria-label as used in Moodle 3.8 and earlier, and the Sharing Cart seems to be looking for the old property. This issue now seems to apply to topics within a page too (unless topics have recently been moved by drag and drop, which mistakenly adds the aria-label property back in--MDL-69637). I think this new one might be a Sharing Cart issue really, because I think it may also apply to Topics/Weekly formats in Moodle 3.9. I don't have Moodle 3.9 at hand to check at the moment, but will give it a try at work. [EDIT: Just checked, and Sharing Cart does seem to have this issue with Moodle 3.9. Now reported this as Sharing Cart issue 61]

This could be reverted in Multitopic 2.0, though, by changing renderer.php function section_header(), before or after the line: 'aria-labelledby' => "sectionid-{$section->id}-title", adding back in: 'aria-label'=> get_section_name($course, $section), This would leave an issue that copying a section to the Sharing Cart after renaming it would still use the old name, I think.

As to the original cause, it seems that Sharing Cart starts out by looking for the section name HTML tag as indicated by the property data-itemtype="sectionname". Page-level sections in Multitopic are marked as not to be linked, and instead have data-itemtype="sectionnamenl" ("nl" for no link), which isn't itself unusual--Topic/Weekly formats do this in One-section-per-page layout--but they seem to have an extra, hidden, section name HTML tag marked data-itemtype="sectionname" instead. I guess that makes this one a Multitopic format issue.

One way that seems to work to fix this in Multitopic is just not mark page level sections as not to be linked. In renderer.php function section_header(), on the line starting with: $sectionname = removing the part: $section->levelsan >= FORMAT_MULTITOPIC_SECTION_LEVEL_TOPIC && This would create the issue that there is now a link on the page that just links to the page itself, and it doesn't seem to be caught by the JavaScript I have for handling collapse/expand, or ignoring clicks on non-collapsible sections.

I notice you've got a fork of Multitopic format, so I expect you can apply these changes to your server to get Sharing Cart working in the meantime, if you're OK with the related issues? I think I'd rather look into it a bit more, so see what's involved in working around these, when I have time, before applying changes myself.

Thanks, James

james-cnz commented 4 years ago

Hi Brad,

I believe this is fixed in the current version. Let me know if not, otherwise I'll close the issue in a few days.

Cheers, James