gjbarnard / moodle-format_topcoll

Collapsed Topics course format for Moodle.
http://docs.moodle.org/en/Collapsed_Topics_course_format
GNU General Public License v3.0
35 stars 60 forks source link

Moodle 4.0: Reactive state/course index not updating on section renaming #121

Closed PhMemmel closed 2 years ago

PhMemmel commented 2 years ago

When changing the name of a section via the "inplaceedit" feature there is no reactive state change triggered.

Result: The course editor is not being updated to the new section name. Also other plugins might rely on this great new feature and want listen to such a change.

Tested on current master of this plugin (28da3804022b2f0e724879a68898a004daee3793) and MOODLE_400_STABLE

gjb2048 commented 2 years ago

@PhMemmel Ok, thanks for the info, how do I solve it please?

gjb2048 commented 2 years ago

@PhMemmel And indeed replicate and test this in vanilla Moodle 4.0.

PhMemmel commented 2 years ago

Unfortunately, the new courseeditor things aren't documented yet and I am no format developer, so I have to make a lucky guess here: Probably you would have to react to the YUI event saving the section name change, prevent the bubbling and call the corresponding section name change method of the courseeditor and let it do the rest. This should properly trigger both the ajax call to change the name and the reactive state change. But that's just a shot in the dark, sorry

gjb2048 commented 2 years ago

@PhMemmel Ok, ta. Umm, as far as I can see, I'm implementing all of the PHP for inplace editable, that uses the core mustache, that calls the core JS for this sort of thing, so currently stumped.

PhMemmel commented 2 years ago

Maybe you can figure out, how topics format is handling this (it works there) and check for difference. Apart from that that might be question for the telegram dev channel as long as there isn't any documentation for the courseeditor online yet.

gjb2048 commented 2 years ago

@PhMemmel Already looking an trying to find exactly where the Moodle react event chain is broken. Markup seems correct. Inplace editable working, so perhaps strange that the react event firing code is not in the same place - but this is a hunch at the moment. Topics is firing an extra AJAX update, looked at that but nothing stands out yet. Will need to see how the react base code works and actually fires events to be processed by the watchers.

PhMemmel commented 2 years ago

Found the problem, PR incoming

PhMemmel commented 2 years ago

For solution see linked PR

gjb2048 commented 2 years ago

@PhMemmel Thanks, I had tracked it down to 'initCoursePage' in /course/amd/src/actions.js not being run.

gjb2048 commented 2 years ago

@PhMemmel Really thank you, appreciated.

gjb2048 commented 2 years ago

I wonder if ajax_section_move() is needed too?

PhMemmel commented 2 years ago

You're welcome!

PhMemmel commented 2 years ago

I wonder if ajax_section_move() is needed too?

not sure, but the code currently contains this method already anyway? But I could imagine that it is not needed anymore, because it might be rerendered from JS side directly from mustache template

gjb2048 commented 2 years ago

Ah, that's why I have course_section_updated() in the renderer. Will have to see if ajax_section_move() is called when that action happens.