Add reset functionality #6
Conversation
|
Looks great, awesome stuff :) |
| $(this).on('click'); | ||
| }); | ||
|
|
||
| for (i = 0; i < this.options.summaries.length; i++) { |
There was a problem hiding this comment.
i is implicitly declared here, making it global
|
I've had the chance to look at the code a bit, and it looks very promising! 👍 The functionality that has been added and the code looks really good, however it conflicts a bit with some existing functionality that you may not be familiar with: Try this:
Also the 'retry'-button is not added when resuming Summary if you left off at the last page. You also might want to have a look at contracts that IV and CP expects when 'retry' is implemented: https://h5p.org/documentation/developers/contracts. Other content types implement a behaviour that lets you enable/disable the 'retry' button. Soon there! :) |
|
Thanks for the feedback @thomasmars, I've finally gotten around to making some changes:
Hopefully, second time lucky! |
| this.showSolutions = function() { | ||
| // intentionally left blank, no solution view exists | ||
| this.getAnswerGiven = function () { | ||
| if (this.progress > 0){ |
There was a problem hiding this comment.
can be simplified to return (this.progress > 0);
|
Looking a lot better!
When these issues have been solved I think its ready to go! :) |
|
Fixed the highlighted issues. I've added a semantics boolean for enabling the retry button with a default value of false. I think I'll create an upgrade.js script in another pull request as I still need to wrap my head around how upgrading h5p content types work. |
| behaviour: { | ||
| enableRetry: true, | ||
| }, | ||
| enableRetry: true, |
There was a problem hiding this comment.
You have moved the enableRetry variable out of the behaviour group, which means that it no longer satisfies the contract to parents: https://h5p.org/documentation/developers/contracts. The solution could be to create a group for behaviour in the semantics, or make sure that you override enableRetry if it received a behaviour.enableRetry from parent.
|
Nice, I have left a comment for you in the commit regarding contracts, this should be the last thing that has to be fixed :) |
|
Thanks Thomas 👍 I think I removed it because I didn't see the point of having a behaviour group with only one variable but forgot about the contract. I've definitely learnt a lot more about how the content types work together :D |
|
That's good for the JS file, but you also have to change it in semantics, to reflect the structure that you expect in your JS. |
|
Ah, should've realized that I had to do that. I was wondering if errors get thrown if the semantics don't match up with the extended options in the js file? |
|
No, the semantics define the json structure of the params you receive in your JS. You can do whatever you want with them. |
|
Thanks again for the explanation, Thomas 👍 |
|
Awesome, this looks really good now! :) |
|
Ok, let's wait until that bug/hack gets resolved then, or until additional behaviours are added :) |
Added functionality to reset H5P summary content type once the tasks have been completed.
Created and registered a JoubelUI button to handle the reset logic.