feat(theme-classic): themeConfig navbar/footer logos accept className/style + update Meta Open-Source Logo #7643
Conversation
| href: 'https://opensource.fb.com', | ||
| style: { | ||
| maxWidth: '500px', | ||
| }, |
There was a problem hiding this comment.
temporary, we can remove it later if we agree on something on the Infima side
There was a problem hiding this comment.
Why inline style, though? Why not CSS + className?
| @@ -148,8 +148,11 @@ const config = { | |||
| alt: 'Meta Open Source Logo', | |||
| // This default includes a positive & negative version, allowing for | |||
| // appropriate use depending on your site's style. | |||
| src: 'img/meta_opensource_logo_negative.png', | |||
| src: '/img/meta_opensource_logo_negative.svg', | |||
There was a problem hiding this comment.
Do we need this leading slash?
There was a problem hiding this comment.
no :D
With/without it's the same afaik, and historically many config elements don't have one
But I'd prefer to always have one in the future to make it more explicit it's not a relative path. Does it make sense?
There was a problem hiding this comment.
Absolute paths, IMO, are always relative to the domain. The fact that we automatically prepend the base URL is quite magical. But I'm fine with either since it's not strictly a relative path either.
There was a problem hiding this comment.
yes 🤪 I know, no ideal choice there. I'd prefer to have leading / everywhere, but this is just my personal opinion, and Docusaurus v1 did not have any leading / in docs/configs either so 🤷♂️ . I can revert it if needed.
@yangshun you remember why historically we usually don't have any leading /?
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
|
Size Change: +197 B (0%) Total Size: 802 kB
ℹ️ View Unchanged
|
# Conflicts: # packages/docusaurus-theme-classic/src/theme/Logo/index.tsx
Pre-flight checklist
Motivation
For logo customization flexibility, we should be able to provide className or inline styles directly from the theme config
This is also a good workaround for the Infima logo maxWidth being currently a bit too opinionated (see facebookincubator/infima#260)
After Infima upgrade, we can use its sensible default footer logo sizing (30rem) which should be fine for most sites
cc @zpao @yangshun
Test Plan
test on our own website + config validation unit tests
Test links
Deploy preview: https://deploy-preview-7643--docusaurus-2.netlify.app/
On desktop:
On mobile:
Related issues/PRs
facebookincubator/infima#260
#7639