Skip to content
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

Fix various bugs in the Bloom Player implementation #12752

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Oct 25, 2024

Summary

  • Does small clean up of unneeded conditional logic in the bloom player vue component to allow bloomd files to be rendered
  • Fixes previously unnoticed bug where URLs with spaces in them, or URI encoded URLs would not get properly replaced by kolibri-zip - adds regression tests to ensure behaviour
  • Migrates all the bloom player specific code that handles mapping audio files into the bloom player code, use the extensibility of the kolibri-zip module to allow this.
  • Move the HTML style attribute handling into the main kolibri-zip DOMMapper
  • Also fix URL spaces in names and URI encoding issues for DOM parsing too
  • Update the DOMMapper to handle URLs in inline <style> blocks
  • Do a small refactor to make code more readable, and give slight performance improvement on small DOMs
  • Use URLSearchParameters to handle passing options to the bloom player iframe
  • Add more parameters to hide the internal toolbar (defer to our own toolbar)
  • Update bloom player to the latest version from upstream
  • Include a small styling patch to make the previous/next buttons smaller
  • Add to hashi README to describe how to update the bloom-player

References

Fixes #12728
Fixes #12726
Fixes #12725
Fixes #12727

Reviewer guidance

Verify that bloom reader is:

  • still functioning as expected
  • bloomd files are now renderable
  • the internal bloom player controls are hidden
  • all images are properly displayed inside books (when compared with epubs)
  • the previous/next buttons are now a more reasonable size when hovered.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added DEV: renderers HTML5 apps, videos, exercises, etc. DEV: frontend labels Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment