Adds OpenJS Footer #8577
Adds OpenJS Footer #8577
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8577 +/- ##
==========================================
+ Coverage 74.95% 75.01% +0.05%
==========================================
Files 103 103
Lines 9037 9053 +16
Branches 312 312
==========================================
+ Hits 6774 6791 +17
+ Misses 2261 2260 -1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (2)
➖ Removed Assets (2)
|
| "trademarkPolicy": "Trademark Policy", | ||
| "trademarkList": "Trademark List", | ||
| "cookiePolicy": "Cookie Policy", | ||
| "security": "Security Policy" |
There was a problem hiding this comment.
The retention of the security policy here means it is ADDED to the OpenJS footer. I think this is acceptable, as we purposely put it there at one point to expand visibility.
There was a problem hiding this comment.
This file is the only place the new writing is placed. It's slightly duplicative between legal paragraph and the links which are used later, but I think it's acceptable considering alternatives
Should there be a max width? on large screens it does look a bit weird, but fine leaving as is |
| "containers": { | ||
| "footer": { | ||
| "legal": "Copyright <openjsf>OpenJS Foundation</openjsf> and Node.js contributors. All rights reserved. The <openjsf>OpenJS Foundation</openjsf> has registered trademarks and uses trademarks. For a list of trademarks of the <openjsf>OpenJS Foundation</openjsf>, please see our <trademarkPolicy>Trademark Policy</trademarkPolicy> and <trademarkList>Trademark List</trademarkList>. Trademarks and logos not indicated on the <trademarkList>list of OpenJS Foundation trademarks</trademarkList> are trademarks™ or registered® trademarks of their respective holders. Use of them does not imply any affiliation with or endorsement by them.", | ||
| "links": { |
There was a problem hiding this comment.
Why we need these individual keys?
There was a problem hiding this comment.
The links? The diff is hard to see if you are point at line 5 or line 6.
The openjs guidance is a paragraph with links and a list of links afterwards, which makes for duplication...somewhere. I've chosen to put the duplication in the translation strings rather than the code
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR implements the OpenJS Foundation best practices for website footers by restructuring the footer into three sections: a primary section with version badges, a secondary section with social media links, and a new legal section containing copyright text and policy links.
Changes:
- Restructured Footer component to support a new 'legal' slot alongside existing 'primary' and 'secondary' slots
- Added new WithLegal component to render OpenJS Foundation copyright text with embedded links and a list of policy links
- Updated footer navigation configuration with additional OpenJS Foundation links (Terms of Use, Bylaws, Trademark List, Cookie Policy)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/src/Containers/Footer/index.tsx | Restructured footer layout into two rows, added legal slot support |
| packages/ui-components/src/Containers/Footer/index.module.css | Added styling for new row-based layout and legal section with responsive design |
| apps/site/components/withLegal.tsx | New component to render legal paragraph with rich text links and footer links list |
| apps/site/components/withFooter.tsx | Integrated WithLegal component into footer legal slot |
| packages/i18n/src/locales/en.json | Added legal paragraph text and new footer link translations |
| apps/site/navigation.json | Updated footer links with new OpenJS Foundation policy links |
| pnpm-workspace.yaml | Added @nodejs/doc-kit to onlyBuiltDependencies list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bmuenzenmeyer
force-pushed
the
openjs-footer
branch
from
7bb974b to
aa4f93e
Compare
apps/site/components/withLegal.tsx
Outdated
| <p> | ||
| {footerLinks.map((link, index) => ( | ||
| <span key={link.link}> | ||
| <Link href={link.link}>{link.text}</Link> |
There was a problem hiding this comment.
These links seem to lack any hover styling at all? Why can't we use the same white text w/ background on hover that we had before?
There was a problem hiding this comment.
we can, if only there was a way to cascade the styles universally 😄
There was a problem hiding this comment.
🤔 Why can't we use <NavItem> for these, rather than <Link>, so they match the old style + the style used in the top nav (and for the social icons)?
The inline links in the first paragraph make sense to use <Link>, but these standalone links I feel we could do something nicer w/ <NavLink>.
There was a problem hiding this comment.
this might be visually more interesting. i am a bit reluctant to deviate from the design, however.
all like like AS IS
There was a problem hiding this comment.
I do think it would look nicer with the proper nav link styling. Might be worth checking with @kj-powell if there are any concerns here? I can't think there would be as all the links are still there in the correct order, we're just changing the visual presentation to be more consistent with the brand of the site.
There was a problem hiding this comment.
agree. i'm fine either way, but also don't want to open the door to too much derivation.
3 participants
Description
Expands the footer to now have three main sections.
Our top-bar that has version info on the left and social info on the right
A bottom section devoted to legal information
Validation
https://nodejs-org-git-openjs-footer-openjs.vercel.app/en
Related Issues
closes #8574
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.