-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HFP-4136 Use H5P.Column for content #41
base: master
Are you sure you want to change the base?
Conversation
Closing this for now. Removing Accordion from Column options doesn't work for newly created content and I need to figure out why first. |
ad825c1
to
c374bac
Compare
Thank you for your response! Let's connect with them to see what we can do to get this feature implemented :D Or do you reckon we could fork this into it's own content type? 🤔 |
@eSrem They will take care of this pull request as soon as they find some time. They are busy finishing the H5P OER Hub and an update for Branching Scenario for release - and possibly a couple of other things. |
@otacke thank you for your update! I just read the forum update and am happy to read they have the PR's on the agenda :D. |
Hello @otacke so I cloned your repository and switched to add-h5p-column and packed it to an h5p file so I can add it as a library to Moodle where we have H5P already added. but as soon as I try to upload it I get "Invalid H5P content type" error message. |
a) How did you pack it? Probably manually with zip (instead of using h5p-cli as one should) but not setting the required flags - tons of related posts to that on the H5P forum. b) You are aware that this has not officially been accepted by the H5P core team and that using that branch therefore may wreck your Accordion contents in the future, right? |
@otacke first let me thank you for your help and efforts, that so nice of you. |
@AdemOch Flags are only required when you zip manually. I then assume you tried to upload the |
@otacke I highly doubt it since when I switched to the master branch and packed the h5p file it worked fine and accordion was added and when I switched back to this branch the error happens when uploading the packed h5p file, in any case I am trying to do it through the path: Dashboard/Site administration/H5P/Manage H5P content types |
@AdemOch And you have included the editor library that's required? See note above in the pull request? |
@AdemOch You don't need to change anything. Just also clone https://github.com/otacke/h5p-editor-accordion and pack both repositories with H5P-CLI; https://h5p.org/h5p-cli-guide#packcmd |
@otacke Thank you!! that worked so the trick is to upload the editor library first and then it works. |
@AdemOch Not quite. That works, too, but if you create a library file and cannot be sure that the target system has all the required libraries available, then you pack all the libraries into the single h5p library file that contains everything. See https://h5p.org/h5p-cli-guide#packcmd or try |
427d11f
to
4831fe3
Compare
When this pull request is merged in, H5P.Accordion will use an instance of H5P.Column for each panel, thus vastly increasing the versatility as requested multiple times (especially for images). Please see e.g.
There's a separate pull request to only allow images instead of Column content. Please see #33 and choose the one you prefer (none is hopefully not an option) :-)
Please note
H5P.Column could contain H5P.Accordion instances of a different version than the version of H5P.Accordion that H5P.Column is put into. So far, support for running two different versions of the same content type in one H5P content has not been added - and having Accordions inside Accordions might be anyway. Therefore, an additional mandatory editor widget was created for H5P.Accordion that removes H5P.Accordion from the list of options in H5P.Column instances. It can be found at https://github.com/otacke/h5p-editor-accordion
Future options
Question type contract
H5P.Accordion can now host question types, but itself does not implement the question type contract, so for instance will not allow to retrieve xAPI data.
If required, this feature could be provided as well. Please let me know.
Pause/resume media
H5P.Accordion can now host multiple media instances that might play, but can be hidden when collapsing the panel the media is playing in. Media would continue playing, which might lead to confusion, especially if other media are started.
If required, some pause/resume feature could be provided as well. Please let me know.
Since H5P.Column is used in H5P.InteractiveBook which might have the same issue when moving from chapter to chapter, I suggest to add a pauseMedia function (pause all media and remember their state - optionally also for getCurrentState) and a resumeMedia function (use remembered state if available and resume media in that case) to H5P.Column. It could be called by other libraries, e.g. H5P.Accordion and H5P.InteractveBook.