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

Add ContentModal to display resources within the context of a Custom Channel Renderer #8040

Merged

Conversation

jonboiser
Copy link
Contributor

@jonboiser jonboiser commented Apr 29, 2021

Summary

  1. Adds a ContentModal component (which is similar to ContentPage), and integrates it to the CustomContentRenderer component (which renders the custom channel's HTML5 App)
  2. Adds a themeRenderer method to the window.kolibri object provided by hashi
  3. Adds version getter to window.kolibri
  4. Adds a themeValidator utility to sanitize themes provided by the custom channel app and, if needed, provide warnings or errors. Uses the tinycolor2 library to parse/manipulate colors.
  5. Moves files to /Learn/CustomChannel for organization
  6. Integrates the option added in Add KOLIBRI_ENABLE_CUSTOM_CHANNEL_NAV - default True to options.py #8025 to TopicsPage
  7. Fixes an issue where updating the context query param would cause a refresh

Demo of modal with custom theme and responsive behavior

CleanShot 2021-04-29 at 14 37 08

References

Fixes #7832
Fixes #7833

Reviewer guidance

  1. To enable custom channels make sure to
    export KOLIBRI_ENABLE_CUSTOM_CHANNEL_NAV=True
    export KOLIBRI_CENTRAL_CONTENT_BASE_URL=https://hotfixes.studio.learningequality.org
    before running the devserer
  2. Download the example channel with token favim-zinul
  3. Interact with the custom channel.

If possible, add a new custom channel with a different theme and see how the theme validation works.

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

  • Automated test coverage is satisfactory
  • 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

@jonboiser jonboiser added this to the 0.15.0 milestone Apr 29, 2021
@jonboiser jonboiser added the TODO: needs review Waiting for review label May 3, 2021
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a code read through and looks good so far. I'm going to do another round of manual QA in the morning and one more read through tomorrow with a fresh brain, and see if I can test out some of the custom themes also. @rtibbles if you have time for a quick review also to see if I missed anything, then I can go ahead and approve after I go through the manual QA in the morning 😃

Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the version() change, this looks good to me! Nice work @jonboiser. @rtibbles is going to do one more pass to make sure I didn't miss anything, but looks like we are ready to demo tomorrow ✅ 🥳

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing jumped out at me during a read through.

@rtibbles rtibbles merged commit 79ca8e4 into learningequality:develop May 4, 2021
@jonboiser jonboiser deleted the custom-channels-theming branch May 12, 2021 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
3 participants