Mon, 04/10/2017 - 09:54
Mon, 04/10/2017 - 11:32
Is there any suggestions for improvement especially about the code structure, since I am a beginner here...
Thu, 04/13/2017 - 13:08
I am not an expert myself, but I have had a look at your code, and I have some general suggestions ...
I have some more proposals, but major things first ;-)
Tue, 04/18/2017 - 08:14
Do you have an account on Github, where you can share your code (makes it easier to review it).
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.
Tue, 04/18/2017 - 15:20
Thank you for sharing your content type. I have had a look at the code - here's my feedback:
Let us know if anything is unclear - we'll help!
Tue, 09/26/2017 - 16:31
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
Wed, 09/27/2017 - 09:30
Thanks for the feedback, Jimmy. The best way for this to be picked up by the Jithin, the creator of the content type, is to create an issue at the Github repository where the source code lives:https://github.com/jithin-space/h5p-ImageSequencing
Note that you will have to create an account on Github if you don't have one already.
Sat, 04/22/2017 - 11:53
Thanks otacke,tomaj and fnoks for your feedbacks..
I am progressing on standardizing my code as per your feedbacks..I will upload the next version through github as soon as it is finished.. I am very excited to see the feedbacks and thanks for spenting time on my work and for your helping mentality...
Mon, 04/24/2017 - 07:34
Happy to help! Good luck with your coding!
Tue, 04/25/2017 - 15:44
could anyone please tell me how to use joubelUI in my content type...I am getting TypeError: H5P.JoubelUI is undefined....
Here is what I did,
please tell what I am missing?
Wed, 04/26/2017 - 08:59
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:
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
Wed, 04/26/2017 - 11:26
Here is the link to my repository...it is not fully working...library json and scripts/image-sequencing.js are the files in consideration...
I beleive h5p.JoubelUI is installed in my drupal setup
Thu, 04/27/2017 - 06:56
Thu, 04/27/2017 - 09:18
I didn't get the idea at first...I just uploaded the attachment, use its url in the post and finally deleted the post..I think it is being displayed now
Sun, 04/30/2017 - 15:32
It is working now...but I am not able to exactly what caused that..Anyway thanks for the help..
Fri, 05/05/2017 - 14:13
My Code for H5P.ImageSequencing Content type is being updated. I tried to my best to make use of all your suggestions..It is now properly indented, translatable, resizeable and structured.. Please go through the code and tell me if there exist any more modifications..
A screen shot of how a content created using that type is also being attached...
sorry on my previous problem existed on a development branch and I shared the link for the master branch..
Tue, 05/09/2017 - 09:40
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)
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)
Tue, 05/09/2017 - 11:57
Tomaj: This happens because you are not including H5P.Timer. Nothing wrong with the content type :)
Tue, 05/09/2017 - 11:46
Great that you're making progress! I think there are still some kinks to be ironed out, e.g. some global variables that should be avoided, because they can easily be overwritten. In my humble opinion, an awesome tool for getting some hints is JSHint. I embedded it in atom recently, and I love it for showing me possible issues instantly when typing my code.
Tue, 05/09/2017 - 14:14
I see great improvements in your code. It is much tidier, and the way you have divided it into components makes it easy to follow. Great work!
I have done some testing, and especially when the images are of different height/width ratio, the UX-experience is a little strange, since the dropzones are jumping around while dragging. Generally, I think it is a little bit hard to know what I am supposed to do when seeing this content type the first time. If you'd like, I could get a UX designer to suggest a design for this content type - just let me know! I did find a bug when dropping one of the images outside the dropzones, then the image disappeared.
Below you'll find some comments about the code:
Your CSS is setting styles in a generic way (e.g. html, h1, h2, h3, h4, dd) and some of the selectors are too generic. This will be a problem since this content type is included using a DIV-tag, and those CSS-rules will then apply to other elements on the page. The best solution is probably to configure this H5P to be included using an iframe instead. This is done by adding the following to your library.json:
A drawback of displaying the H5P inside an iframe is that you need to inform the parent window about any resizing. This is done by triggering the resize event on the main class (ImageSequencing in your case). Eg: self.trigger('resize');
There are still some untranslatable strings in dropper.js ("Drop Here", "please drop")
Somewhere you are setting CSS rules using JS. This should not be necessary unless the values are dynamic/calculated. Instead, you should toggle classes. Here is an example:
self.$dropper.css('border', '2px solid red');
Could e.g instead be:
and your CSS could actually set the style:
border: 2px solid red;
Thu, 05/25/2017 - 14:02
Now I am again with new code because the previously detected bug needed a detailed restructuring.
Please Once Again test my Code and make your suggestions...
I am happy to get the suggestions also from a experienced UX designer as fnoks had mentioned...
Mon, 06/05/2017 - 12:00
I am not getting any responses for the past two weeks...please respond so that I can move forward onto more h5p tasks..sorry if you were too busy!!!!
Tue, 06/06/2017 - 09:02
Our UX-designer has given some feedback, which is available here:https://h5ptechnology.atlassian.net/browse/HFP-1156
Thu, 06/15/2017 - 07:17
Thanks for the feedback from the ux part. I would like to implement the changes as suggested. Could you please provide the design specs as mentioned in the response so that I can implement it quickly..
Tue, 07/04/2017 - 08:52
Code Once again updated based on the suggestions from the ux designer. Major changes include , switch back to H5P jquery ui for smoother sortable functionality and removal of resize event...I think it works good automatically in all screen sizes...Please review the code ..
Thu, 02/28/2019 - 05:14
Does anyone know how to resize the images in this awesome content type? I tried to make changes in the h5p-image-sequencing.css to make the image smaller than 168 px, but it didn't do anything. Really appreicate any insights.
Connect with H5P
H5P is a registered trademark of Joubel