fix(theme): remove hardcoded fill from Bluesky and LinkedIn icons #11407
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Thanks
I think we let you override the colors, but we should preserve the default colors we initially set?
Also worth mentioning that there are other icons that may be affected by this problem (StackOverflow, YouTube, Twitch, Mastodon...), and setting a global fill: grey for all social icons may not always work. I'm fine fixing these 2 only if you don't care about the others for now.
| x: sebastienlorber | ||
| linkedin: sebastienlorber | ||
| github: slorber | ||
| bluesky: sebastienlorber.com |
There was a problem hiding this comment.
This is our example, not supposed to be edited manually
It is generated from our templates, so it will be overriden on next release.
It would be better to edit our template instead: https://github.com/facebook/docusaurus/blob/main/packages/create-docusaurus/templates/shared/blog/authors.yml
I'm fine if you add my Bluesky account here, but it is already on our website authors anyway, so if you run our website the icon was already there:
packages/docusaurus-theme-classic/src/theme/Icon/Socials/Bluesky/index.tsx
Outdated
Show resolved
Hide resolved
| d="M218.123 218.127h-37.931v-59.403c0-14.165-.253-32.4-19.728-32.4-19.756 0-22.779 15.434-22.779 31.369v60.43h-37.93V95.967h36.413v16.694h.51a39.907 39.907 0 0 1 35.928-19.733c38.445 0 45.533 25.288 45.533 58.186l-.016 67.013ZM56.955 79.27c-12.157.002-22.014-9.852-22.016-22.009-.002-12.157 9.851-22.014 22.008-22.016 12.157-.003 22.014 9.851 22.016 22.008A22.013 22.013 0 0 1 56.955 79.27m18.966 138.858H37.95V95.967h37.97v122.16ZM237.033.018H18.89C8.58-.098.125 8.161-.001 18.471v219.053c.122 10.315 8.576 18.582 18.89 18.474h218.144c10.336.128 18.823-8.139 18.966-18.474V18.454c-.147-10.33-8.635-18.588-18.966-18.453" | ||
| fill="#0A66C2" | ||
| /> | ||
| style={{'--dark': '#000', '--light': '#fff'} as CSSProperties} |
There was a problem hiding this comment.
Same here, we lost the LinkedIn default icon color
There was a problem hiding this comment.
Restored the blue color in light mode, but also used the exact brand shade LinkedIn uses in dark mode (they drop blue shade in favour of slightly transparent white).
There was a problem hiding this comment.
Looks good to me, but I think you inverted the colors:
I think the convention we used on the GitHub icon is not super intuitive: --dark means we want a dark icon color, and not that we are currently in dark mode. We want a dark icon when we are in dark mode 😅.
Not sure what would be the most intuitive, but if we change that convention, I'd rather normalize the practice over all the social icons.
Yeah, saw that too, but some of those icons include gradients or multiple colors which would be a bit more work to properly overwrite and allow customization without changing the SVG. Since we did not use them on the React Native website, I decided to leave them as is for now, but can look at this later, in my free time. |
slorber
left a comment
There was a problem hiding this comment.
LGTM apart from the linkedin colors being inverted.
We can handle the other icons later if anyone care.
Pre-flight checklist
Motivation
On React Native website we have spotted out that Bluesky social icon used for blog authors have a hardcoded fill color, refs:
This also can be seen by running
classic-typescriptexample located within monorepo:How
In this PR, I have refactored the Bluesky and LinkedIn icons components (spotted the problem with LinkedIn while testing) to match other themable social icons code.
I have also added
blueskyprofile to Seb's social lists inclassic-typescriptexample to be able to verify the changes.Test Plan
The changes have been tested by running
classic-typescriptexample locally.Preview