Attemptfeedback as html wysiwyg editor

[ralferlebach]

[vtos]

@ralferlebach Thank you for the input, however, this cannot be accepted in its current form, as it's too straightforward.

We should anticipate how the decision to enable/disable the custom feedback text will be made in the plugin's code. Currently, it looks into the field's text to check whether it contains anything. If so, it's a flag to display the custom feedback text. If it's empty, the plugin considers custom feedback is 'off'. This will not work when having an HTML editor field, however. Because even when visually the editor may look empty, when saved, the relevant property may contain something like this in the database: <p></p><p></p><br />. This will be considered as a non-empty value by the code and it'll try to display the custom feedback text, though visually it'll be empty. Yes, we could use some cleaning functions for such case, but it still looks unreliable and 'hacky'. A more solid solution would be having another property/form field like 'Enable custom feedback' holding the flag of whether the activity creator wants custom feedback to be displayed. Besides, we must also think how the existing instances will migrate to using this approach on the sites where the adaptive quiz activity has been used for a long time already.

Actually, such feature has already been considered by the plugin maintainer, but was postponed due to having more pressing concerns in the plugin development. It's still on the radar, and, perhaps, will follow immediately after the 3.0.0 release containing the custom CAT models functionality. I'll keep this one open to post updates or link related commits here.

Also, when providing a pull request, squash all commits into just one commit will all the changes. The commit should have a descriptive message following Moodle's code style regarding the commit message formatting.

Regards Vitaly

[ralferlebach]

Dear Vitaly,

thank you for your detailed feedback and insightful thoughts on my PR. Please allow myself to investigate all aspects and come up with solutions until end of next week. As you wrote, a WYSIWYG solution for that field would improve your plugins user value, so I am deeply convinced personally, the gains will justify remaining efforts with ease.

Will that be fine for you?

Of course I will also keep an eye on compatibility with existing data and settings, if necessary by employing mechanisms in update.php, as by your wish.

One important information I will need from you, when also addressing the aspect of output: are there any changes intended on the attempt's feedback pages yet? If not, I will also use the opportunity to make the transition to mustach templating at these locations as well.

Will this be fine for you?

Best regards Ralf

PS: For the cat model of Wunderbyte, we need the moodle context created before the hook is called anyway. So this solution seems to hit two birds with one stone and benefit both projects at the same time.

[vtos]

@ralferlebach Thank you for the suggestion, as far as I remember, I've already refactored this page in the version with custom CAT models, this is the upcoming 3.0.0 which is about to be released. But, this refactoring won't be contained in 3.0.0, I'm planning to include it in one of the further releases (say, 3.1.0).

One thing though, can you please elaborate on this -

PS: For the cat model of Wunderbyte, we need the moodle context created before the hook is called anyway. So this solution seems to hit two birds with one stone and benefit both projects at the same time.

Does that mean you need an extra parameter in the API, that is, a context instance? If yes, it's the right moment to include it in 3.0.0 now before it's released.

[vtos]

@ralferlebach Just an addition to my previous message - here's the commit with the refactoring - https://github.com/vtos/moodle-mod_adaptivequiz/commit/d1704e2bec01b7bbeae7475131ccf682dd0be7df So, doesn't make sense for you to get to it. That commit will be merged eventually (more likely, in 3.1.0 as mentioned previously).