h5p / h5p-accordion

Accordion content type for H5P
MIT License
4 stars 67 forks source link

Use H5P.Column for content #41

Open otacke opened 4 years ago

otacke commented 4 years ago

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 https://github.com/h5p/h5p-accordion/pull/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.

otacke commented 4 years ago

Closing this for now. Removing Accordion from Column options doesn't work for newly created content and I need to figure out why first.

eSrem commented 3 years ago

Hi @otacke how's it going? I'm interested in this pull request and #33 as they both seem to reflect using H5p.Column as the accordion options.

How can we help getting these pull requests into h5p master finalised?

otacke commented 3 years ago

@eSrem #33 did not use H5P.Column but merely H5P.Image ans was closed. I fear you'll have to ask the H5P core team, if there's something that could be done - that's out of my control.

eSrem commented 3 years ago

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? 🤔

otacke commented 3 years ago

@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.

eSrem commented 3 years ago

@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.

AdemOch commented 2 years ago

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.

otacke commented 2 years ago

@AdemOch

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?

AdemOch commented 2 years ago

@otacke first let me thank you for your help and efforts, that so nice of you. a) I did use the h5p-cli to pack it, I saw that you already bumped up the minor version compared to the one I had so I didn't do anything, so what flags do you think I should also set up before packing, I think I saw the H5P forum but didn't see someone who had the same issue. b)Yes I am well aware

otacke commented 2 years ago

@AdemOch Flags are only required when you zip manually. I then assume you tried to upload the .h5p file in the H5P Hub. That won't work. You're holding library file in your hands, not a content type file. That library file will have to be uploaded on the H5P library settings screen.

AdemOch commented 2 years ago

@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

otacke commented 2 years ago

@AdemOch And you have included the editor library that's required? See note above in the pull request?

otacke commented 2 years ago

@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

AdemOch commented 2 years ago

@otacke Thank you!! that worked so the trick is to upload the editor library first and then it works.

otacke commented 2 years ago

@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 h5p help pack.