-
Notifications
You must be signed in to change notification settings - Fork 647
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
Bloom player #12586
base: develop
Are you sure you want to change the base?
Bloom player #12586
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things from the previous review have been addressed: #12531 (review) but the way the mapper is being referenced and applied is not quite fitting in with how kolibri-zip operates. A couple of other questions/clarifications.
<body style="background-color: #2e2e2e;"> | ||
<div id="root"><span style="color: #d65649">Loading Bloom Player...</span></div> | ||
<script type="text/javascript" src="bloomPlayer-469de4b6cca7dc5c3e04.min.js"></script></body> | ||
<!-- At build time, we insert a script tag pointing at bloomPlayer.min.js but with a hash created at build time, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment seems at odds with the script tag above?
Where is the CSS loaded from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSS are loaded using the DOMWrapper
to avoid getting a stale version from a cache (while allowing us to cache it for a long time). | ||
It has a style sheet which sets the same color on the body, but it takes a while to fetch and load, | ||
so we reduce flicker by setting a background color explicitly. --> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on the binary files below - but are the mp3 files referenced in the JS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, mp3 files are being used in js files to play audio for correct and wrong answers
initBloom() { | ||
try { | ||
this.loaded(); | ||
this.iframe.src = `../bloom/bloomplayer.htm?url=${this.contentUrl}&distributionUrl=${this.distributionUrl}&metaJsonUrl=${this.metaUrl}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
Now we can save the progress for bloom format files using the pages viewed |
This currently adds no new JS dependencies, so I would suggest reverting any changes to the |
Summary
Added a new plugin for bloom format files and created a runner in the hashi to render bloom files in the iframe client.
Version 1:
https://github.com/user-attachments/assets/1d1d73f8-b685-4acd-a68a-0a03ae7657cc
Version 2:
https://github.com/user-attachments/assets/2eb5ffc8-b47e-42e5-9b96-22a0b2b58882
Version 3:
Who.am.I.mp4
Joseph.the.Dreamer.mp4
…
References
#12237
#36
#12531
…
Reviewer guidance
…
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)