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

feat: theme-agnostic view to fetch theme assets #29503

Merged

Conversation

regisb
Copy link
Contributor

@regisb regisb commented Dec 3, 2021

It is possible to set custom logos in microfrontends, for instance with the
LOGO_URL setting. Ideally, we would like that MFEs share the same logos as the
LMS. But this is made difficult when comprehensive theming is enabled, and the
logo is overridden by a custom theme. In that scenario, the logo url becomes
/static/mytheme/images/logo.png. But the MFEs do no know that the "mytheme"
theme is enabled. To resolve this issue, we introduce here a view, at the
"/theming/asset/" url, that redirects to the corresponding asset in the
theme that is currently enabled. Thus, MFEs only have to set
LOGO_URL=http://lmshost/theming/asset/images/logo.png to point to the themed
logo.

Related issue: overhangio/tutor-mfe#25

This is a cherry-pick of the following PR: https://github.com/edx/edx-platform/pull/29461

It is possible to set custom logos in microfrontends, for instance with the
LOGO_URL setting. Ideally, we would like that MFEs share the same logos as the
LMS. But this is made difficult when comprehensive theming is enabled, and the
logo is overridden by a custom theme. In that scenario, the logo url becomes
/static/mytheme/images/logo.png. But the MFEs do no know that the "mytheme"
theme is enabled. To resolve this issue, we introduce here a view, at the
"/theming/asset/<path>" url, that redirects to the corresponding asset in the
theme that is currently enabled. Thus, MFEs only have to set
`LOGO_URL=http://lmshost/theming/asset/images/logo.png` to point to the themed
logo.

Related issue: overhangio/tutor-mfe#25
@regisb regisb requested a review from a team December 3, 2021 09:57
@openedx-webhooks openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Dec 3, 2021
@openedx-webhooks
Copy link

Thanks for the pull request, @regisb! I've created OSPR-6253 to keep track of it in JIRA.

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

Copy link

@edx-community-bot edx-community-bot left a comment

Choose a reason for hiding this comment

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

👍

When you're ready to merge, add a comment that says

@edx-community-bot merge

and we'll handle the rest!
CC: @edx/community-engineering @edx/open-release-maintainers

@regisb
Copy link
Contributor Author

regisb commented Dec 3, 2021

Can a non-bot maybe approve this? cc @BbrSofiane

@pdpinch pdpinch merged commit b234f5c into openedx:open-release/maple.master Dec 3, 2021
@openedx-webhooks
Copy link

@jmbowman, @kdmccormick: thought you might like to know that regisb merged this pull request.

@openedx-webhooks openedx-webhooks added merged and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Dec 3, 2021
@openedx-webhooks
Copy link

@regisb 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@pdpinch
Copy link
Contributor

pdpinch commented Dec 3, 2021

👍

@natabene
Copy link
Contributor

natabene commented Dec 7, 2021

@regisb Thank you for your contribution.

@regisb regisb deleted the regisb/theming-asset-url branch December 8, 2021 08:46
regisb referenced this pull request in overhangio/tutor-mfe Dec 8, 2021
Previously, it was not possible to make the MFEs use the themed logo
from the LMS. This changed when this PR was merged:
https://github.com/edx/edx-platform/pull/29503
In Maple, the /theming/asset/images/logo.png url now redirects to the
themed logo.

Close #25.
regisb referenced this pull request in overhangio/tutor-mfe Dec 14, 2021
- Previously, it was not possible to make the MFEs use the themed logo from the
  LMS. This changed when this PR was merged:
  https://github.com/edx/edx-platform/pull/29503 In Maple, the
  /theming/asset/images/logo.png url now redirects to the themed logo. Close #25.

- We disable New Relic globally by upgrading frontend-build for some packages.
  Close openedx/wg-frontend/issues/14
regisb referenced this pull request in overhangio/tutor-mfe Dec 14, 2021
- Previously, it was not possible to make the MFEs use the themed logo from the
  LMS. This changed when this PR was merged:
  https://github.com/edx/edx-platform/pull/29503 In Maple, the
  /theming/asset/images/logo.png url now redirects to the themed logo. Close #25.

- We disable New Relic globally by upgrading frontend-build for some packages.
  Close openedx/wg-frontend/issues/14
regisb referenced this pull request in overhangio/tutor-mfe Dec 21, 2021
- Previously, it was not possible to make the MFEs use the themed logo from the
  LMS. This changed when this PR was merged:
  https://github.com/edx/edx-platform/pull/29503 In Maple, the
  /theming/asset/images/logo.png url now redirects to the themed logo. Close #25.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants