Opencast-Moodle / moodle-block_opencast

Block to manage Opencast publications in moodle
21 stars 27 forks source link

Proposal: (delete block) removing seriesmapping with confirmation #355

Closed ferishili closed 5 months ago

ferishili commented 6 months ago

This PR is a form of proposal that would fix #351

How it works

NOTE: the wording may need to be adjusted and prettified.

Tutorial Video

https://github.com/Opencast-Moodle/moodle-block_opencast/assets/53179227/08a9b7d5-30a4-4eb4-8db6-10cf4e1cec9e

justusdieckmann commented 6 months ago

Hey @ferishili, my apologies for the delay.

Thank you very very much for your hard work and giving us options to choose from! We like this approach, but would rather have one popup instead of two.

I created a very quick (non-functional) popup prototype to see if the idea was any good: image

What is your oppinion? I would gladly add a commit to this PR to merge the two popups. If you would like to do the remaining programming instread, I can push my (very much) WIP commit somewhere.

Thank you again! Justus

ferishili commented 6 months ago

Hey @ferishili, my apologies for the delay.

Thank you very very much for your hard work and giving us options to choose from! We like this approach, but would rather have one popup instead of two.

I created a very quick (non-functional) popup prototype to see if the idea was any good: image

What is your oppinion? I would gladly add a commit to this PR to merge the two popups. If you would like to do the remaining programming instread, I can push my (very much) WIP commit somewhere.

Thank you again! Justus

Hi @justusdieckmann, hi @NinaHerrmann, Happy new year! No problem at all and thanks for your feedback. About adding options as button in that dialog, I am not sure if it could follow the moodle standards, or we need to provide some hacky-workarounds! And I assume that we have to follow Moodle Standards according to @NinaHerrmann! However, please go ahead and use this PR and add/change/enhance and make your changes. Thanks

justusdieckmann commented 5 months ago

Hey,

happy new year to you too! This would be my attempt.

Screencast from 11.01.2024 19:03:49.webm

It requires the manageseriesforcourse capability to be shown the popup. In #356 we change the capability so that teacher have that by default on new installations. Also we will suggest giving teachers that capability in the next release notes.

I used the unlink_series external-api call to remove the series from the course before deleting the block, since it seemed to be the way with the least amount of extra boilerplate code, but I could of course also do it in a different way.

What is your opinion on the general approach?

Cheers Justus

ferishili commented 5 months ago

Hey,

happy new year to you too! This would be my attempt.

Screencast.from.11.01.2024.19.03.49.webm It requires the manageseriesforcourse capability to be shown the popup. In #356 we change the capability so that teacher have that by default on new installations. Also we will suggest giving teachers that capability in the next release notes.

I used the unlink_series external-api call to remove the series from the course before deleting the block, since it seemed to be the way with the least amount of extra boilerplate code, but I could of course also do it in a different way.

What is your opinion on the general approach?

Cheers Justus

Hey, Thanks for your work, well done, that is a good improvement. This seems good to me and could fulfil the need in one modal.

BR