Skip to content
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

Chore: (design systems rework) #343

Merged
merged 26 commits into from
Jul 20, 2020
Merged

Chore: (design systems rework) #343

merged 26 commits into from
Jul 20, 2020

Conversation

jonniebigodes
Copy link
Collaborator

This pr will address the required changes to the design systems tutorial.

As this is a work in progress, for the time being this will be in draft mode as it will be constantly being updated, once it's ready it will be moved from draft so that it can be fined tuned and merged.

@jonniebigodes jonniebigodes marked this pull request as draft July 8, 2020 15:48
@jonniebigodes jonniebigodes marked this pull request as ready for review July 9, 2020 21:44
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Made some small suggestions

content/design-systems-for-developers/react/en/review.md Outdated Show resolved Hide resolved

![Button component changed in deployed site](/design-systems-for-developers/netlify-deployed-site-with-changed-button.png)
![Button component changed in deployed site](/design-systems-for-developers/chromatic-deployed-site-with-changed-button.png)

For each component and story that changed, copy the URL from the browser address bar and paste it wherever your team manages tasks (GitHub, Asana, Jira, etc) to help teammates quickly review the relevant stories.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domyen / @jonniebigodes do we think we should talk about Chromatic's UI review feature at this point, if only because otherwise people may be confused about the fact that we are describing a workflow here that is different to that, and thus we will confuse them?

Maybe at this point we should just add an aside:

Chromatic also offers a complete UI Review workflow built into the product as part of its paid offering. The technique of copying Storybook links into a GitHub PR works at a smaller scale (and with any service that hosts your Storybook, not just Chromatic), but as your use increases you may consider that services as it automates the process.


First, go to [chromatic.com](https://chromatic.com) and sign up with your GitHub account.
In the <a href="https://www.learnstorybook.com/design-systems-for-developers/react/en/review/#publish-storybook">previous chapter</a> we learned how to publish Storybook using [Chromatic](https://www.chromatic.com/). We added a bold red border around each `Button` component and then requested feedback from teammates.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In the <a href="https://www.learnstorybook.com/design-systems-for-developers/react/en/review/#publish-storybook">previous chapter</a> we learned how to publish Storybook using [Chromatic](https://www.chromatic.com/). We added a bold red border around each `Button` component and then requested feedback from teammates.
In the <a href="review/#publish-storybook">previous chapter</a> we learned how to publish Storybook using [Chromatic](https://www.chromatic.com/). We added a bold red border around each `Button` component and then requested feedback from teammates.

I think we should use relative links?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonniebigodes mentioned they didn't work quite right in Gatsby for some reason. I can confirm that I get a routing error when I use the relative link.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we can make some kind of link that doesn't include a domain work

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmeasday the reason we went with including the full url for this two situations was due to the fact that Gatsby+ Markdown does not "like" that. When you add a link in markdown relative to another page with a heading with will bubble up to the top of the page. I've tested it and it will always default to the top of the page.

We could go about this in two ways:

  • Add the gatsby-remark-autolink-headers plugin to the site and we have a way use the links/navigation better.
  • Manually update both gatsby-ssr and gatsby-browser like it's used in the plugin to include the navigation.

Me personally i'm more inclined to the first one. As at some point it could be extremely beneficial to include a TOC per chapter and proper heading linking, which will be extremely more beneficial for the translations.

A initial approach was put together here, per @domyen feedback i thought it over and it would be better to address this properly somewhere in the near future

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've created a issue (#347) for tracking the usage of links in headings here. We can revisit it once we have a little more breathing room.

Sounds good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK!

@@ -338,7 +398,7 @@ addDecorator(story => (

You’ll now be able to browse the design system components and docs while developing the example app. Showcasing the design system during feature development increases the likelihood that developers will reuse existing components instead of wasting time inventing their own.

Alternatively, you can browse your design system’s Storybook online if you deployed it to a web host earlier (see chapter 4).
Alternatively, you can browse your design system’s Storybook online if you deployed it to <a href="https://www.learnstorybook.com/design-systems-for-developers/react/en/review/#publish-storybook">Chromatic </a> earlier (see chapter 4).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use relative link

@domyen domyen merged commit 809ea9c into master Jul 20, 2020
@domyen domyen deleted the chore_reword_design_systems branch July 20, 2020 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants