-
Notifications
You must be signed in to change notification settings - Fork 86
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 #391
Conversation
Mind taking a look at the tests? Looks like we're running into some related failures. |
65fbc7c
to
d4518dd
Compare
@arbrandes I pushed a fix for the tests. |
Holding off here for a https://github.com/openedx/frontend-slot-footer implementation. |
9e5234f
to
d8e8e6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to merge once the comment is addressed!
@@ -24,7 +24,7 @@ const App = () => ( | |||
/> | |||
</Routes> | |||
</main> | |||
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind also removing the variable from https://github.com/openedx/frontend-app-gradebook/blob/master/.env.development#L10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d8e8e6b
to
13a19a2
Compare
Note: This removes the ability to use
LOGO_POWERED_BY_OPEN_EDX_URL_SVG
to set the logo in the footer. This was directly usingprocess.env
and therefore did not support the more robust configuration methods provided by https://github.com/openedx/frontend-platform/blob/master/src/config.js. The footer component itself supports usingLOGO_TRADEMARK_URL
to set the logo (and supports the more robust configuration methods).What changed?
Developer Checklist
Testing Instructions
[ How should a reviewer test this PR? ]
Reviewer Checklist
Collectively, these should be completed by reviewers of this PR:
FYI: @openedx/content-aurora