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

feat(react): first round of playground updates #12209

Closed

Conversation

TannerS
Copy link
Contributor

@TannerS TannerS commented Oct 1, 2022

Closes

Hopefully multiple sub issues in #10751

Side note: I had questions about these issues which i asked in a comment in #10751 which may affect/cause me to have to do changes in this pr. I have been having issues understanding what to keep, what to add and how to add it. So please take a look at those questions and let me know what I can fix/change in this PR

Changelog

New

  • Added some new playgrounds for some components

Changed

  • Change some new playgrounds for some components and text and id fixes

Testing / Reviewing

  • Reviewed in storybook

@TannerS TannerS requested a review from a team as a code owner October 1, 2022 05:10
@netlify
Copy link

netlify bot commented Oct 1, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 02f3207
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6337cbb7fe18670008d5b69d
😎 Deploy Preview https://deploy-preview-12209--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 1, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 02f3207
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6337cbb7cd5ab70008d6c92c
😎 Deploy Preview https://deploy-preview-12209--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@abbeyhrt
Copy link
Contributor

abbeyhrt commented Oct 3, 2022

Thanks for doing this @TannerS, this is amazing and so helpful for us! It looks like we have an open PR for loading storybook controls, do you remind removing that one to avoid conflicts?

@TannerS
Copy link
Contributor Author

TannerS commented Oct 3, 2022

@abbeyhrt can you point me in direction of what your referring too?

Also anyway y'all can check on my questions :)

Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

This looks great! I only have one comment about the FileUploader story and loading but everything else is great!!

@@ -20,6 +20,44 @@ import {
import './FileUploader-story.scss';

const filenameStatuses = ['edit', 'complete', 'uploading'];
const argTypes = {
Copy link
Contributor

Choose a reason for hiding this comment

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

FileUploader is a bit tricky since it has such robust examples but I think we would still want it to follow the pattern of the other components in terms of controls, where only the "Playground" story has controls. To do so, if you don't already know, we would want to remove the passed in args from every story besides that one and only define the argtypes for Playground as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abbeyhrt can you explain this a bit more from every story besides that one and only define the argtypes for Playground as well.

Copy link
Contributor

@aledavila aledavila left a comment

Choose a reason for hiding this comment

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

Hey tanner I actually have a PR with the correct playground story. We are cleaning up the stories to where only Playground stories will have the controls and Scott has a Loading PR already up for that as well. Also could you change the -story.js files to be .Stories.js so that they all match and are updated to the file type. And the files must be removed from the next folder and into the root Component file replacing the necessary files. the index files for those components will also need to be updated accordingly so that the Feature flag is removed since it is no longer necessary.

@TannerS
Copy link
Contributor Author

TannerS commented Oct 5, 2022

Closing to start over with new instructions and to prevent overlap with Loading story and file uploader story PRs

@TannerS TannerS closed this Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants