Quick feedback about h5p.shortcut

[DegrangeM]

Thanks for your feedback, I fixed a lots of issues !

Redefining the keys seems unintuitive to me. Why not add a "clear" button?

Why not. But what should happen if we click again in the first input and press keys ? Nothing ? Append ? Replace ?

I think thought of others solutions :

• add a "Set shortcut" button

• onclick it focus the first field and listen for keypress

• on focusout the "set shortcut" became clean shortcut

• onclick clean shortcut button clean the input and transform the button into a set shortcut button

• it's now possible to write things inside the first input like a normal field. It's only when pressing the set shortcut button that it listen to keypress untill we leave the input

or

• add a "Set shortcut" button

• onclick it clean the first field, focus it, and listen for keypress

• it's now possible to write things inside the first input like a normal field. It's only when pressing the set shortcut button that it listen to keypress untill we leave the input

What do you think ?

key

key is a technical term ? How do you call the physical thing on your keyboard that you press to type ?

H5P.Question

Will check it. I found it too late so I skipped it but it might be a good things to use it if I want my content type to be included inside other content type.

Thanks.

[DegrangeM]

I am a little sad that the https://github.com/h5p/h5p-boilerplate/tree/question-type isn't in the h5p tutorial. This would have helped me a lot :/

[otacke]

@DegrangeM I was just thinking of a button to reset the chosen keys - but I am not a UX genius. Anything that makes it more clear how to do that is welcome.

A key is a key, I think :-) It's even in the press any key meme ...

And yes, the boilerplate templates are a nice thing (need some small tweaks, but still). You'd have to contact the H5P core team in order to suggest to add those to the tutorials. Nothing that I can do in that department.

[DegrangeM]

@otacke What do you think the new UI ?

[otacke]

@DegrangeM I like the new editor much better now.

Some more things I noticed now (still didn't do a proper review though ...):

• You're not specifying the embedType in library.json (https://h5p.org/library-definition#libembedtypes), so H5P will put its content into a div instead of into an iframe - and stylesheets may collide. At least on my Drupal instance you lose the cohesive look of H5P contents, because a different font will be used, the default font size is smaller, etc. If you prefer to keep it in a DIV, you should at least try to set properties that themes often set themselves, so your content matches the default set in the general H5P styles.
• If you decide to use an iframe, your click handler for the start button needs to trigger a resize, or the content may be cut off at the bottom, because the iframe wouldn't know that it needs to be resized. That's be done by adding self.trigger('resize'); at the end of the handler (https://github.com/DegrangeM/H5P.Shortcut/blob/master/h5p-shortcut.js#L65-L66).
• Your hidden modifier for the successText style class does only hide the div which will still use up the space that's reserved for the element. Why not use display:none?
• Your results class is never hidden and always uses up min-height: 50px. Why didn't you hide that one as well?
• You're triggering the resize on jQuery at https://github.com/DegrangeM/H5P.Shortcut/blob/master/h5p-shortcut.js#L144 and https://github.com/DegrangeM/H5P.Shortcut/blob/master/h5p-shortcut.js#L151 - but you probably want to call than on the content type itself (thus on self here).

[DegrangeM]

Thanks for the update.

Your hidden modifier for the successText style class does only hide the div which will still use up the space that's reserved for the element. Why not use display:none?

Your results class is never hidden and always uses up min-height: 50px. Why didn't you hide that one as well?

I did it to avoid iframe resizing as we can know since the start the final height. I thinks it's better when we can do that because resizing things are always anoying but I might be wrong. If you think I should do a real hide and then resize tell me you have move experience in h5p.

[otacke]

@DegrangeM I don't have any say in this - I am not part of the H5P core team. Still, I think you will get that feedback from them anyway. And in my personal opinion, presenting empty space that's just a placeholder is way uglier.

[DegrangeM]

I will switch to resize then. The reason I hate resize is because I had trouble with the course presentation activity : when you add QCM for exemple everything seems perfect, then you try it and when the solution appear it is hidden behind a scroll bar so you have to edit your activity to try to increase the height and test again to see if it's now okay :D (but my content will not be embedable in course activity so it's not an issue :p (maybe one day if I add the question contract).

I am not part of the H5P core team

Did you used to be ? Because you are still listed in at least twice webpage (https://h5p.org/the-core-team & https://snordian.de/#snordian)

[DegrangeM]

You're triggering the resize on jQuery at https://github.com/DegrangeM/H5P.Shortcut/blob/master/h5p-shortcut.js#L144 and https://github.com/DegrangeM/H5P.Shortcut/blob/master/h5p-shortcut.js#L151 - but you probably want to call than on the content type itself (thus on self here).

Thanks ! I was wondering why the resize always seems to not working, even if the code was from the documentation (will make a comment to fix that).

[otacke]

@DegrangeM Yes, I used to work for Joubel. https://h5p.org/the-core-team is just (very) outdated, and https://snordian.de/#snordian might in fact be misleading - I'll change that.

[DegrangeM]

• Using the H5P.Question library would make your life a lot easier regarding buttons, xAPI, feedback, score bar, etc. There's a boilerplate template at https://github.com/h5p/h5p-boilerplate/tree/question-type that might have saved you quite some time and also makes implementing the question type contract (https://h5p.org/documentation/developers/contracts) and the save content state feature easy

Note to myself : https://h5p.org/comment/41142#comment-41142

As Degrange suggested, you could have a look at the boilerplate template. If you're not using JavaScript's class notation, then you should check https://github.com/h5p/h5p-true-false/blob/master/scripts/h5p-true-false.js#L487-L489 and https://github.com/h5p/h5p-true-false/blob/master/scripts/h5p-true-false.js#L41-L42 since you had a look at TrueFalse anyway.