H5P-Nodejs-library icon indicating copy to clipboard operation
H5P-Nodejs-library copied to clipboard

Published 20 hours ago • verified publisher icon Lumieducation

feat(theme): add themes

Open JPSchellenberg opened this issue 3 years ago • 6 comments

JPSchellenberg avatar Dec 03 '21 08:12 JPSchellenberg

It would also make sense to accomodate scenarios in which you have a per-content object theme that is configurable by the content author.

sr258 avatar Dec 06 '21 13:12 sr258

There are still details in content type editors that aren't styled: (didn't look at all of them yet)

Bildschirmaufnahme_Auswahlbereich_20220226182608 Bildschirmaufnahme_Auswahlbereich_20220226182621 Bildschirmaufnahme_Auswahlbereich_20220226182634 Bildschirmaufnahme_Auswahlbereich_20220226182700 Bildschirmaufnahme_Auswahlbereich_20220226182715 Bildschirmaufnahme_Auswahlbereich_20220226182750 Bildschirmaufnahme_Auswahlbereich_20220226182804 Bildschirmaufnahme_Auswahlbereich_20220226182817 Bildschirmaufnahme_Auswahlbereich_20220226182914 Bildschirmaufnahme_Auswahlbereich_20220226182948

sr258 avatar Feb 26 '22 17:02 sr258

I've fixed most of the issues above in the editor. There's mostly work to be done in the course presentation editor (it's various submenus).

I've noticed that it seems like a bad idea to override the color property of the body as this has many side effects. H5P is so complex that it's difficult to predict where there might be a panel with a hardcoded background color, so setting the color globally is dangerous as it can easily lead to situations in which there's white on white. It would probably be better to set the color in a more restricted way.

I haven't really looked at the player yet.

sr258 avatar Feb 28 '22 07:02 sr258

I've reviewed the state of the theme in the player and I think the approach of the PR isn't working out. Nearly all content types are broken in some way. I've reviewed about 40% (results below)

I suggest this:

  • We have a list of content types that are supported (separate for editor and player).
  • There are two themes, one for editor, one for player
  • The CSS of the theme is never global but always scoped to the content type (in the player, and as far as possible also in the editor)
  • The H5PEditor and H5PPlayer object decide whether to add a theme to the UI based on the content type
  • There's a callback that implementations can pass in to disable themes for content (so that it is a setting for the content object)
  • We go through the list of content types one by one and write SCSS for it, checking every feature and option. We avoid duplication using SCSS imports and mixins.

@JPSchellenberg @jankapunkt What do you think of this approach? Cornell 1 Cornell 2 Dictation Dictation 2 Drag and Drop Essay 1 Essay 2 Fill in the Blanks Find the hotspot Find the words Guess the answer Image Choice Image Pairing Image Sequence image slider Interactive Video 1 Interactive Video 2 Interactive Video 3 Interactive Video 4 Interactive Video 5 Kewar Multiple Choice Multiple Choice 2 personality quiz 1 Personality quiz 2 Sort Paragraphs Summary Timeline True false Virtual Tour 1 Virtual Tour 2

sr258 avatar Jun 17 '22 14:06 sr258

Wow, @sr258 you made an effort here. From my perspective this sounds good, especially with the scoping. We also use scss but I wonder if there are people who use just css or things like tailwind. Can they still use the theming then?

jankapunkt avatar Jun 17 '22 15:06 jankapunkt

Yes, the theme builder compiles it to css.

sr258 avatar Jun 17 '22 15:06 sr258