-
Couldn't load subscription status.
- Fork 12
Adding Get Started docs for set up instructions #2809
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
Conversation
🦋 Changeset detectedLatest commit: 27b55eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: 0 B Total Size: 109 kB ℹ️ View Unchanged
|
88b7c15 to
7fc823e
Compare
7fc823e to
c9f26e1
Compare
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-xlbcpftmxr.chromatic.com/ Chromatic results:
|
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.
Let me know if I'm missing anything, or feel free to update this as well!
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 got me thinking. Do you think it would make sense to keep this guide here? or would it be better to move it to Confluence?
I'm not sure if it's hard for product engs to find these pages in Confluence and if it is better fit to keep these docs in Storybook instead.
For example, we have these simple instructions here: https://khanacademy.atlassian.net/wiki/spaces/WB/pages/2217541905/Installing+Web+Wonder+Blocks+in+your+local+environment, but I'm not sure if people have checked that page and if it would make sense to expand it if we see fit for it.
How do you feel about all of this? would love to hear your thoughts.
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.
My assumption was that WB documentation is easier to find in Storybook since that is where devs go to find information on how to use components or tokens! I feel like the type of docs in our Confluence space (except for the release notes) are more about how to work on WB!
One thing I like about having technical documentation in Storybook is that the docs are searchable with the rest of the codebase! If we have to make updates to docs based on a code change, sometimes we are able to find related docs when searching in the repo, while searching and updating docs in Confluence is an additional step. For example, in these set up instructions, I added a step for installing peer dependencies which includes aphrodite. When we migrate away from aphrodite, we can search for the term "aphrodite" and it'd be a reminder we can update the getting started docs too!
What are your thoughts on this? @marcysutton too in case you have thoughts on where docs like this should live!
Sidenote: I'm able to view analytics in Confluence and see 9 unique viewers all time on the doc you shared! Though, they are instructions for setting up the WB repo, rather than installing WB in another project! I'd be curious to know if we have access to any analytics for our Storybook docs!
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.
Thanks for the info! I think that's a totally fair observation.
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.
question: How do you feel about adding the CodeSandbox link here so folks still can see the steps in an IDE?
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'm weary of adding the CodeSandbox link to the docs since it would be an additional thing we'd need to maintain! I set it up mostly as a draft for testing 😄
It has crossed my mind though that it would be nice to have a WB environment to easily try out components (whether it be in CodeSandbox or something else!). I'm not sure what the proper way to do that is yet. I used my GitHub account on CodeSandbox to make the draft project (I was able to install WB packages since they are public on npm!)
| } | ||
| ``` | ||
|
|
||
| 2. More details to come on how to import fonts! |
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 was taking a look at how the fonts were set up for thunderblocks in webapp: https://github.com/Khan/webapp/pull/32961
I saw we added the related font files and defined the css font faces in webapp/frontend. I'm wondering if it would make sense to include those in WB so that consumers don't need to set these things up in their project. And we could modify any fonts internally within WB. Let me know what you think!
For now, I've included a placeholder for these steps and created WB-2102 to fill in more details about importing fonts!
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 valid point! I think it would be a good idea to encapsulate fonts within WB :).
__docs__/_get-started_.mdx
Outdated
| ## Design | ||
|
|
||
| The design system components are available in Figma: | ||
| - <a href="https://www.figma.com/design/EuFu0U7gqc1koXm8ZhlOLp/%E2%9A%A1%EF%B8%8F-Components?node-id=1-2">Wonder Blocks Component Library (Thunderblocks)</a> |
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.
Using anchor tags for links so that they don't open in the Storybook iframe
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.
One thing that I've noticed is that sometimes we get requests from external people to see our WB Figma files. I wonder if we should instead remove these links and move them to another place (maybe Confluence?).
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.
Good question! We can create a Confluence page about the Figma files and link to that instead!
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 created a Wonder Blocks Resources page in Confluence so we can link to that instead! https://khanacademy.atlassian.net/wiki/spaces/WB/pages/4396089552/Resources
I updated Figma links in the docs to this page instead 😄 Thanks for the suggestion!
| ```css | ||
| html { | ||
| font-size: 62.5%; | ||
| } | ||
| ``` |
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.
Kinda related to the other comment: https://github.com/Khan/wonder-blocks/pull/2809/files#r2396094476
I wonder if we could also include setting the root font size in a base WB stylesheet so that if we ever need to change this, we can change it in one place within WB!
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.
Good point! I think it would be safe to do it with our current sites. But I'm not fully sure how impactful would be with other places that combine WB with other styles/components. Maybe it would be better to let the consumer decide where to put that override (e.g. which selector).
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.
Maybe it would be better to let the consumer decide where to put that override (e.g. which selector).
If I'm understanding correctly, consumers will need the font-size: 62.5% on the root html element in order for the correct rem values to work! Or is it fine for the root font-size to be a different value, since the system should scale according to it?
It is interesting though that the base rem value is defined outside of WB so WB isn't the thing that defines that 1rem is 10px (the apps or Storybook preview config determines it!)
(There aren't any issues with how things are now so this can be a consideration for later!)
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: NOT Published🤕 Oh noes!! We couldn't find any changesets in this PR (da46aac). As a result, we did not publish an npm snapshot for you. |
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 all looks great! thanks so much for adding this missing part in our docs 👏
Hopefully we can engage more with engineers with this useful information.
| Please note – before contributing ensure that any design changes you are wanting | ||
| to make are reflected in the [Wonder Blocks project in | ||
| Figma](https://www.figma.com/file/VbVu3h2BpBhH80niq101MHHE/Wonder-Blocks). | ||
| Figma](https://khanacademy.atlassian.net/wiki/spaces/WB/pages/4396089552/Resources#Figma). |
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.
praise: Thanks for updating this!!
| } | ||
| ``` | ||
|
|
||
| 2. More details to come on how to import fonts! |
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 valid point! I think it would be a good idea to encapsulate fonts within WB :).
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.
question: How do you feel about adding the CodeSandbox link here so folks still can see the steps in an IDE?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2809 +/- ##
============================
============================
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary:
Setting up documentation around how to set up Wonder Blocks in an application.
Note: I've created WB-2102 for writing instructions for importing fonts!
Issue: WB-1892
Test plan:
?path=/docs/get-started--docsI set up an example in code sandbox to figure out what steps are needed to set up WB: https://codesandbox.io/p/sandbox/wb-set-up-4k4n6v