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 #391

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. This was directly using process.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 using LOGO_TRADEMARK_URL to set the logo (and supports the more robust configuration methods).

What changed?

  • [ More in depth breakdown of changes ]
  • [ Peripheral things that got changed ]
  • [ etc... ]

Developer Checklist

  • Test suites passing
  • Documentation and test plan updated, if applicable
  • Received code-owner approving review
  • Bumped version number package.json

Testing Instructions

[ How should a reviewer test this PR? ]

Reviewer Checklist

Collectively, these should be completed by reviewers of this PR:

  • I've done a visual code review
  • I've tested the new functionality

FYI: @openedx/content-aurora

@arbrandes
Copy link
Contributor

Mind taking a look at the tests? Looks like we're running into some related failures.

@brian-smith-tcril
Copy link
Contributor Author

@arbrandes I pushed a fix for the tests.

@arbrandes
Copy link
Contributor

Holding off here for a https://github.com/openedx/frontend-slot-footer implementation.

@brian-smith-tcril brian-smith-tcril force-pushed the footer-slot branch 4 times, most recently from 9e5234f to d8e8e6b Compare May 16, 2024 19:40
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.

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} />
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.

@arbrandes arbrandes merged commit d9a0a11 into openedx:master May 17, 2024
5 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.

2 participants