Image Sequencing

Hi Jithin!

I am not an expert myself, but I have had a look at your code, and I have some general suggestions ...

• You might want to have a look at the coding guidelines of H5P. For example, you should not use 4 spaces but 2 spaces for indenting. This is basically a matter of taste, but keeping it consistent will help others to understand H5P code across different source files.
• Don't forget to add comments to your code. They should explain why you did something the way you did rather than translating your code to human language. This will not only help others to understand your code, but also yourself in 6 month or so ;-)
• In semantics.json, you might want to use a "list" instead of a "group" for uploading images, see https://h5p.org/semantics#type-list. This wouldn't only make your code more flexible, but also shorter, and people won't have to upload all images again if they decide they could need one more image for the sequence.
• You shouldn't use a JavaScript alert to show the score, I think. Why not have a look at some other content types that use the progress bar with the star for feedback?

I have some more proposals, but major things first ;-)

Hi Jithin,

Do you have an account on Github, where you can share your code (makes it easier to review it).

For writing good code, I recommend reading Clean code JavaScript.

In general I would recommend splitting up your attach method, to make it easier to read. Specially moving the call backs out to seperate class methods.

- Tom

Hi,

Thank you for sharing your content type. I have had a look at the code - here's my feedback:

• You should not use an alert box. You could e.g. use the score bar, which is part of H5P.JoubelUI (github.com/h5p/h5p-joubel-ui).
• It does not handle window resizing.
• Usage of jquery UI: should probably use the jquery-ui H5P library instead. Or even better: don't use jquery UI at all :)
• Semantics.json is unnecessary complex. Just use a list with min:3, max:6.
• Use two spaces for indentation
• Variable names are not consistent:

1. Variables prefixed with $ should be jQuery objects
2. Don't use underscore in variable names. E.g. instead of e_width use eWidth;
3. Variables like "N" should not be used, since it doesn't say what it is.

• Labels are hardcoded, i.e. this content type is not translatable
• Some of the DOM elements have ids that will not be unique if several Image Sequencing H5Ps are placed on the same page. I do not see they are in use, so they probably could be removed? E.g.: dragZone, dropZone and titleZone

Let us know if anything is unclear - we'll help!

Great job on this content type, thanks!  I had a friend test this on his ipad and when he taps/drags the image, only the image drags, NOT the "card" with the text and the image.  If he taps and drags the "text" portion, the whole card works as expected.  Not sure if this can be fixed, but thought I'd share just in case!

Thanks again for sharing your work

Happy to help! Good luck with your coding!

- Tom

It looks like you have done the correct thing, i.e added a dependency to JoubelUI, which means that library will be loaded when your H5P content type is loaded. Have you put the dependency in the right place? I.e, it should be inside preloadedDependencies:

"preloadedDependencies": [
  {
    "machineName": "H5P.JoubelUI",
    "majorVersion": 1,
    "minorVersion": 2
  }
]

If this is what your library.json looks like, you should verify version 1.2 of H5P.JoubelUI is installed on your system!

Let us know how it goes

Hi,

1. Awesome that you got things moving forward :)
2. The image in this blogpost is pointing locally to your computer. A trick that I do, is to use the Attachments field at the bottom of the comment form, to upload an image. And the copy the url of that image to be part of the post itself.

- Tom

I tested Image Sequencing, and I it looks very cool! :)

But I get an exception from the timer in Chrome 58.

timer.js?opbf00:14 Uncaught TypeError: Cannot read property 'call' of undefined

    at new ImageSequencing.Timer (timer.js?opbf00:14)

    at ImageSequencing.self.attach (image-sequencing.js?opbf00:325)

    at Object.H5P.newRunnable (h5p.js?opbf00:821)

    at HTMLDivElement.<anonymous> (h5p.js?opbf00:132)

    at Function.each (jquery.js?opbf00:2)

    at init.each (jquery.js?opbf00:2)

    at Object.H5P.init (h5p.js?opbf00:92)

    at HTMLDocument.<anonymous> (h5p.js?opbf00:2012)

    at c (jquery.js?opbf00:2)

    at Object.fireWith [as resolveWith] (jquery.js?opbf00:2)

- Tom

Hi,

Our UX-designer has given some feedback, which is available here:
https://h5ptechnology.atlassian.net/browse/HFP-1156