-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Light branding for the reference site #65764
Conversation
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @auareyou! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
Size Change: 0 B Total Size: 1.77 MB ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
This is fantastic! Thank you for this excellent improvement. I can think of numerous additional things to follow up on, the first coming to mind is to also use Inter for the content on the right, vs. just for the sidebar. But perhaps that, and other improvements, can be followups?
I'd love a quick gut-check by @mirka and @ciampo if they have time, but otherwise, ship it!
storybook/theme.js
Outdated
brandImage: './wp-org-logo.png', | ||
|
||
// Typography | ||
fontBase: '"Inter", sans-serif', |
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.
I don't think we load Inter
in the Storybook page, double-checking with @mirka — that would mean that only folks who already have it installed locally would actually use it, and everyone else would fallback on the local sans-serif
font
(for example, I could see the Inter font on my machine because it's coincidentally loaded by the Grammarly extension — but when I switched to an incognito window, the text was rendered with the OS's sans serif font)
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.
Correct 👍
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.
Or can use the system font stack:
-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,Oxygen-Sans,Ubuntu,Cantarell,"Helvetica Neue",sans-serif
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.
Changing the font to Inter only affected the sidebar, and even when I changed it in the body
nothing changed in the content. Does anybody know why that would be?
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.
It looks like Storybook has its own font family stack:
According to the docs, the main UI and the docs are themed differently and may need two separate configurations
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.
If it's ok with everyone else, we'll leave this change in the content for the next update, along with your suggestions @mattrwalker and @jasmussen?
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.
Yep, updating the font stack to system fonts can happen any time, doesn't have to be this PR. So long as we remember to follow up! 🚀
Digging how this feels more cohesive and on brand. I'm curious how much effort it would take to adjust some styling in the menu, like using our icons within the sidebar and adjusting some of the active/hover states. I think you (@auareyou) mentioned that doing more in-depth customization requires a lot more effort though. Audi sidebar as an example: |
Color changes and added static directory to swap out logo
65aefce
to
ec947e8
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.
Pushed a couple of updates to fix the CI error and update the logo with a (lossless-ly) compressed version, which shaved off 60% of the file size.
LGTM 🚀
@auareyou , thank you for working on this! Feel free to merge once CI checks have passed
What?
The first batch of changes to customize the Storybook site using their theming API. Some styles require digging into custom CSS and private APIs and are not included in this PR. I also created a static directory local to the storybook configuration for any future static files, and added the new logo there.
Why?
This is one of the initiatives to move our design systems efforts forward and gain more consistency across reference sites.