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

Tutor Indigo Theme Updates #53

Closed
wants to merge 17 commits into from

Conversation

hinakhadim
Copy link
Collaborator

No description provided.

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

Thanks for opening this preliminary PR Hina. I see that we are overriding many html templates. If we do not take some precautions it's going to be extremely difficult to upgrade those in the future. Can we keep our html changes to a minimum? And where changes are needed, clearly label them with an HTML comment.

@hinakhadim
Copy link
Collaborator Author

hinakhadim commented Dec 7, 2023

Regis, We've removed the static files which we haven't overridden. Also, Not a lot of changes we've made but comments has been added by Tanveer where he made changes in the html files.
I've also added the ENV_PATCHES for the learning-mfe. This will install the header, footer and brand-openedx in learning-mfe image.

@regisb
Copy link
Contributor

regisb commented Dec 12, 2023

This is awesome work. I'm eager to see this live.

landing page

When unauthenticated, the "explore courses" menu in the top left corner looks off:

Screenshot from 2023-12-12 10-43-48

favicon

I think we should not use the "" logo for the favicon: it's not neutral enough. Instead, we should use the blue "<>" logo:

Screenshot from 2023-12-12 10-48-53

course about page

In the course "about" page, when I hover on the social sharing links, the "Share with friends and family" modal looks off. The background should not be black, the font should not be serif and not italic:

Screenshot from 2023-12-12 10-49-44

@regisb
Copy link
Contributor

regisb commented Dec 13, 2023

Closed in favour of #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants