-
Notifications
You must be signed in to change notification settings - Fork 7
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
feature/issue 23 Styleguide and Design System #36
Conversation
✅ Deploy Preview for super-tapioca-5987ce ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 a good start, thanks!
I took the opportunity to source the fonts from npm and can confirm it is working both for development and Storybook preview. Formatted the PR description a bit.
I left some feedback to consider but let me know if you need any help on it, I can help with some of them if unclear:
- Limit the design tokens / CSS variables to just the most common / global ones we'll need for now
- Just need a little formatting and links for the Documentation page
- For the Styleguide docs, we'll need to curate the design tokens list and then show examples (like you can see in the reference example)
- For the tech stack docs, just take what you see from the reference link and just remove the Tailwind / Linting mentions (and the part about EventBrite and Netlify Forms since they don't apply to this project)
Will mark this PR as a draft as we get through the feedback items.
src/stories/documentation.mdx
Outdated
## GreenwoodJS Website Documentation | ||
|
||
|
||
We use a combination of technologies to build the GreenwoodJS website |
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.
Looks like the formatting is a bit off here and there aren't any links, so let's make sure to get those in
You can check the source from the linked reference for an example
https://raw.githubusercontent.com/AnalogStudiosRI/www.blissfestri.com/main/src/stories/Documentation.stories.mdx
Also, I think if we can have these side links uppercased (I think we just need to rename the files to Documentation etc, that would be ideal.
src/stories/styleguide.mdx
Outdated
|
||
|
||
## Colors | ||
The following theme colors available are: |
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.
We'll definitely want to flesh this section out with examples like in the reference storybook
https://raw.githubusercontent.com/AnalogStudiosRI/www.blissfestri.com/main/src/stories/Styleguide.stories.mdx
src/stories/styleguide.mdx
Outdated
## Typography | ||
The whole website uses Vercel's **Geist** font in the following sizes: | ||
|
||
`--hero-big` |
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.
For the design system tokens, I don't think the goal is to try and capture every token used throughout all pages like these --hero
specific ones, but rather just the ones all other tokens should build / compose off of.
I think just the main colors, fonts, sizes, etc should be good to start out with. We don't need to predict every token right now, and this can totally evolve and grow as we build out the rest of the pages. 🏃♂️
src/stories/styleguide.mdx
Outdated
|
||
|
||
## Spacing | ||
The whole website is built on an 8pt Grid, and spacing is always in multiples of 8px |
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 we have any tokens for this, it would be good to document them here.
src/styles/theme.css
Outdated
--color-white: #FFFFFF; | ||
--font-primary: 'Geist', 'sans-serif'; | ||
--font-size-base: 16px; | ||
--container-radius: 6rem; |
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.
similar to my comments above, we don't have to capture page / component specific tokens for the design system, we can start with just the ones that apply to broadest parts of the design that should be used everywhere, like colors and fonts. This can always grow as the site grows.
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.
@Auhseh
It would be good if we get as many of the images we think we'll need in now if we're planning on committing a bunch of them. I think at least for the header we would need
- Discord
- X / Twitter
- "App Launcher" (or whatever we call the icon we show when the header navigation is in mobile view)
Added the other assets and also added a short ReadMe with a guide on how to use the assets in code. |
Thanks, it's the Grid background, Renamed it and added context in the ReadMe |
I think everything's pretty much ready, last thing would be to adding Description icons in the "Why Greenwood" section, pending when we articulate the copy. |
Thanks, let me review. Let's try and keep the filename consistent as I had updated them to be something like some-name.png on the last change. I will fix the new ones added but just so we can keep the convention the same for everyone contributing to the project. 👍
Thanks, but I'm going to remove / clean this up since I was just asking as part of this conversation here in GitHub, I don't want to start documenting every single asset, ideally when we start using them their purpose will be clear, or even better we can use a more descriptive name. Same goes for color variables, let me just move any relevant info into the storybook instead and / or apply a name change. I also deleted the Menu.svg since I don't think that actually needs to be an image, just some text and CSS should do fine for that, and I assumed it might be flexible as a general "badge" like component. |
Ok, I was hoping maybe we could handle all those with CSS, but maybe we will need these as SVGs? |
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 looks good! Going to leave it open a couple more days for a final review, then will merge. 🚀
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.
As a side note, if anyone feels comfortable setting up build tools it might be a good time to start contributing on #24 as we start ramping up contributions. Feel free to reach out in that issue if you have any questions about getting started with that task. 🙏
|
||
## Spacing | ||
|
||
The whole website is built on an 8pt Grid, and spacing is always in multiples of 8px |
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 maybe we can just delegate to Open Props
https://open-props.style/#sizes
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.
Will delegate to this higher level conversation, this is a good start for getting things up and running for contributions - #7 (comment)
|
||
* { | ||
font-family: var(--font-primary); | ||
font-size: var(--font-size-base); |
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.
Perhaps we should also add box-sizing
too?
https://css-tricks.com/almanac/properties/b/box-sizing/
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.
Added!
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 great! I'll plan on following up with Linting / Formatting / Contribution Guidelines next and then at least that should allow to make an initial start on our Development Roadmap.
I'll also make sure to get a basic strategy documented around a more formal CSS Strategy as well.
Related Issue
resolves #23
Summary of Changes
TODO
url
properties (like for@font-face
+src
) during CSS file optimization greenwood#1199 (or track unpatching it) - @thescientist13 - upstream feature / fixes tracking #37