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: use frontend-plugin-framework to provide a FooterSlot #345

Merged
merged 1 commit into from
May 17, 2024

Conversation

brian-smith-tcril
Copy link
Contributor

Note: This removes the ability to use LOGO_POWERED_BY_OPEN_EDX_URL_SVG to set the logo in the footer. The footer component itself supports using LOGO_TRADEMARK_URL to set the logo.

Note: There are other PluginSlots in this MFE that have not been moved to the plugin-slots directory and turned into components yet. I wanted to keep the focus of this PR on adding the FooterSlot, so I created an issue to track that effort #344 and will PR those changes separately

@jsnwesson
Copy link
Contributor

Thanks for writing this up, Braden! The @openedx/2u-aperture team and I will be looking over this PR early next week, while also monitoring the conversation for Learning MFE's PR.

@brian-smith-tcril brian-smith-tcril force-pushed the footer-slot branch 4 times, most recently from 0fb1fc6 to 2c5ae41 Compare May 16, 2024 19:42
Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

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

Looks good, though we should remove the env var from all the files before merging.

@@ -109,7 +109,7 @@ export const App = () => {
)}
</main>
</AppWrapper>
<Footer logo={getConfig().LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
<FooterSlot />
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@arbrandes arbrandes merged commit 49f9e6e into openedx:master May 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants