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

Read-only Inputs: Checkbox Implementation #12243

Closed
5 tasks
Tracked by #12260 ...
sstrubberg opened this issue Oct 5, 2022 · 5 comments · Fixed by #12398
Closed
5 tasks
Tracked by #12260 ...

Read-only Inputs: Checkbox Implementation #12243

sstrubberg opened this issue Oct 5, 2022 · 5 comments · Fixed by #12398
Labels
adopter: PAL Work-stream that directly helps a Pattern & Asset Library. component: checkbox needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: enhancement 💡 version: 11 Issues pertaining to Carbon v11
Milestone

Comments

@sstrubberg
Copy link
Member

sstrubberg commented Oct 5, 2022

Reference #12241

image

image

Acceptance Criteria

  • Coded component in /package/react
  • Add a prop for read-only, add styles for said prop
  • Controls for each state in playground
  • Default story for easy copy and paste
  • RTL tests for the read-only class on the prop
@sstrubberg sstrubberg changed the title Checkbox [Read-only Input Components]: Checkbox Oct 5, 2022
@sstrubberg sstrubberg added proposal: accepted This request has gone through triaging and we are accepting PR's against it. component: checkbox needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. and removed status: needs triage 🕵️‍♀️ labels Oct 5, 2022
@sstrubberg sstrubberg changed the title [Read-only Input Components]: Checkbox [Checkbox]: Coded component Oct 5, 2022
@sstrubberg sstrubberg changed the title [Checkbox]: Coded component Read-only Inputs: Checkbox Implementation Oct 6, 2022
@sstrubberg sstrubberg added this to the 2022 Q4 milestone Oct 6, 2022
@lee-chase
Copy link
Member

Personally prefer a control to a story, a disabled story would be odd.

Perhaps also read only specific color tokens e.g. $border-readonly

@lee-chase
Copy link
Member

  • MDX file update?

@lee-chase
Copy link
Member

Q: A little concerned elsewhere we are using readOnly as opposed to the standard attribute readonly. Will this not cause confusion?

@sstrubberg sstrubberg pinned this issue Oct 25, 2022
@tay1orjones tay1orjones unpinned this issue Oct 25, 2022
@tay1orjones
Copy link
Member

tay1orjones commented Oct 25, 2022

@lee-chase I'll jump in and try to answer some of these questions.

Personally prefer a control to a story, a disabled story would be odd.

We apply VRT to each story in the storybook. By having a dedicated story titled "readonly" we can ensure there are no visual regressions over time. I'm really either/or on this one though - we don't usually have specific stories for other states like invalid. This could be a follow-on item later. For now it can be a control on the "playground" story imo

Perhaps also read only specific color tokens e.g. $border-readonly

My guess is no, we won't add these as tokens. Adding tokens has lots of implications across the system. Ultimately this is a question for design though, @aagonzales what are your thoughts?

MDX file update?

Story files should import an .mdx file that corresponds to the "docs" tab in storybook. This .mdx file will need updated to include the readonly story as well as light docs on how/when to use the readonly prop/variant.

Q: A little concerned elsewhere we are using readOnly as opposed to the standard attribute readonly. Will this not cause confusion?

Yeah I think the standard attribute should be used here where possible, following convention of how we use other standard attributes like title. I think it should be listed as a prop and probably just have a proptype of bool.

@aagonzales
Copy link
Member

aagonzales commented Oct 27, 2022

I personally don't think we need read-only tokens. I think using a mix of regular and disabled color tokens in this context is acceptable. If the style ever diverges, we can make a specific one. Its only used in a few read-only components. We could make a whole suite of read-only tokens but I think that is overkill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adopter: PAL Work-stream that directly helps a Pattern & Asset Library. component: checkbox needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. package: @carbon/react @carbon/react proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: enhancement 💡 version: 11 Issues pertaining to Carbon v11
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants