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.

Sign up for GitHub

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

Jump to bottom

docs: add instructions to customise MFE logos #16

Merged
merged 2 commits into from
Nov 12, 2021
Merged

docs: add instructions to customise MFE logos #16

merged 2 commits into from
Nov 12, 2021

Conversation

BbrSofiane
Copy link
Member

@BbrSofiane BbrSofiane commented Oct 18, 2021 •
edited
Loading

Make Logo URLs configurable using config.yml.

See thread.

@BbrSofiane BbrSofiane mentioned this pull request Oct 22, 2021
Closed
@BbrSofiane
Copy link
Member Author

Haven't heard any objections so I will mark this as ready for review.

@overhangio/tutor-developers this is ready for review.

@BbrSofiane BbrSofiane changed the title [DRAFT] Set variables for mfe marketing assets Set variables for mfe marketing assets Oct 27, 2021
@regisb
Copy link
Contributor

regisb commented Oct 28, 2021

This is being discussed here: https://discuss.overhang.io/t/customise-mfes-branding/2029/2

@regisb regisb mentioned this pull request Nov 8, 2021
Closed
@BbrSofiane BbrSofiane force-pushed the bbrsofiane/add-variables-for-logos branch from ab66899 to ccf52ab Compare November 10, 2021 19:39
@BbrSofiane BbrSofiane force-pushed the bbrsofiane/add-variables-for-logos branch from ccf52ab to 3bc5b17 Compare November 10, 2021 19:43
@BbrSofiane BbrSofiane changed the title Set variables for mfe marketing assets docs: add instructions to customise MFE logos Nov 10, 2021
@BbrSofiane
Copy link
Member Author

@regisb I've made the changes to the README.

About improving the default logo, did you have any suggestions? I considered using something similar to your implementation of LazyStaticAbsoluteUrl but that would require changes to the tutor core. Is my assumption correct?

@regisb
Copy link
Contributor

regisb commented Nov 12, 2021

Damn! I keep forgetting that the logo url depends not just on the LMS_HOST, but also on the theme that is in use on the lms.

I think it's a shame that we have to jump through so many hoops just to fetch a simple static asset. I wish there was a view in edx-platform that 301-redirected /theme/images/logo.png to the actual logo url. Maybe we can make such a change upstream? For instance in the "openedx/core/theming" app?

I'll open a PR upstream. In the meantime, I'll merge your change.

@regisb regisb merged commit facceb9 into overhangio:master Nov 12, 2021
@regisb regisb mentioned this pull request Nov 12, 2021
Closed
@BbrSofiane BbrSofiane deleted the bbrsofiane/add-variables-for-logos branch November 15, 2021 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
Tutor project management
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BbrSofiane @regisb