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

Update design system #255

Merged
merged 11 commits into from
Apr 1, 2021
Merged

Update design system #255

merged 11 commits into from
Apr 1, 2021

Conversation

domyen
Copy link
Member

@domyen domyen commented Mar 31, 2021

Changelog

  • Input: remove secondary state and use default state instead
  • Input: update code state based on the styling we use in Chromatic
  • Select: remove secondary and use default state instead
  • Textarea: remove code state
  • Textarea: remove secondary state and use default state instead
  • Add and update comment icons
    image

Note
This requires downstream changes in Chromatic and LearnStorybook to use the new APIs

@domyen domyen requested a review from kylesuss March 31, 2021 21:06
Comment on lines 13 to 14
export const playground = (args) => <Radio {...args} />;
playground.args = { label: 'label', hideLabel: false };
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like we are converging on this pattern of calling these Template in the Chromatic repo & also in the SB docs. I haven't seen this "playground" pattern before. Where'd it come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the stories that used to be called "Basic". They're the "representative" story that you can manipulate with Controls.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also the non-capitalization and export (export playground vs export Playground) goes against this other piece of the docs & pattern we have been following in Chromatic where we do this:

export default {
  includeStories: /^[A-Z]/
}

so that all of the capitalized exports are treated as stories.

Copy link
Collaborator

Choose a reason for hiding this comment

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

These are the stories that used to be called "Basic". They're the "representative" story that you can manipulate with Controls.

That's right. Kinda forgot we had that. Is that a thing we recommend doing with all components these days?

Copy link
Member Author

Choose a reason for hiding this comment

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

Useful for design system components, not sure about creating a playground for more complex components

Copy link
Collaborator

@kylesuss kylesuss left a comment

Choose a reason for hiding this comment

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

Seems good. Nice to get rid of the unused stuff. I think it makes sense to establish patterns that we use across codebases so that's why I called out some things here, but I suppose they aren't blocking. We just started in Chromatic focusing a bit more on emulating what the SB docs recommend which probably makes a lot of sense here since its all open source.

@domyen
Copy link
Member Author

domyen commented Apr 1, 2021

@kylesuss can you review this again? If everything is OK please do a minor release 🙏

@kylesuss kylesuss added the minor Increment the minor version when merged label Apr 1, 2021
Copy link
Collaborator

@kylesuss kylesuss 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! Appreciate you working through the template/playground stuff w/ me.

@kylesuss kylesuss merged commit 3272e56 into master Apr 1, 2021
@kylesuss kylesuss deleted the updated-design-system branch April 1, 2021 15:11
@kylesuss
Copy link
Collaborator

kylesuss commented Apr 1, 2021

🚀 PR was released in v5.5.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants