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

[DO NOT MERGE]: Read only rollup #12601

Closed
wants to merge 94 commits into from

Conversation

lee-chase
Copy link
Member

This relates to #12241

Added this to enable reviewers to get an understanding of the full set of read-only components

Changelog

New

Added a new story `Experimental/ReadOnlyComparrisons

Testing / Reviewing

Checked to story works locally.

@netlify
Copy link

netlify bot commented Nov 11, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 3f5d09c
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/636e45f705e6a700085d38dd
😎 Deploy Preview https://deploy-preview-12601--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 Nov 11, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 3f5d09c
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/636e45f7deabf900086d6b98
😎 Deploy Preview https://deploy-preview-12601--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.

@tay1orjones
Copy link
Member

@lee-chase what did you want to do with this one? Should these changes be merged or marked ready for review?

@lee-chase
Copy link
Member Author

@lee-chase what did you want to do with this one? Should these changes be merged or marked ready for review?

@sstrubberg just nudged me via Slack.

I had intended it as a demo, but when it was discussed I believe Scott or yourself indicated it would be a good idea to keep some of this and simplify how it is built.

I believe we thought it nice to keep the 'All' page could discard the rest. I can certainly update to do that.

One open question would be as to where in the storybook this is placed, it is currently sitting under the Experimental category.

@tay1orjones
Copy link
Member

@lee-chase I think the value derived from the All page will be encompassed by the docs updates to the website listed in #12241 These docs will show the comparison between the different states as it relates to the read-only variant.

Besides that, the readOnly prop was added to all the Playground stories, so consumers do have a way to preview and understand the prop and how it impacts the component's visual and behavior. So overall I think we could close this for now.

I do believe that we should resurrect this once we have a way to add stories that are included for development purposes only, #12971. I'd love to see the All story visually snapshotted so we can avoid regressions in the styles across disabled and readonly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants