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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/update checkbox #40

Merged
merged 18 commits into from Oct 20, 2020
Merged

Feat/update checkbox #40

merged 18 commits into from Oct 20, 2020

Conversation

kaylarobertson3
Copy link
Contributor

@kaylarobertson3 kaylarobertson3 commented Oct 16, 2020

Proposed changes

This PR updates the checkbox component so it's ready for use in the sapera-website forms.

  • Update onChange function to allow components to function inside of a form the same as other Input components
  • Add required, and hasError, improve a11y, and optionally display error message
  • Centers checkbox label to design, update spacing slightly
  • This PR also updates colors in the pattern library with the most recent ones from design. Later, colors will be shared across repo in a css utility repo, but for now, if they update in sapera-website they should also change in sapera-components. (for example, thin will not exist as a border option in sapera-website, although it currently exists. It is not used anymore in this repo since it's being phased out)

Can QA sapera-components locally and test by clicking on the checkboxes in Checkbox story

This PR can optionally be tested in the sapera-website footer newsletter subscription in a related PR (WIP) https://github.com/infographicsgroup/sapera-website/pull/426 by using yarn link

Before

Screen Shot 2020-10-16 at 11 50 57

After

Screen Shot 2020-10-16 at 11 20 52

Screen Shot 2020-10-16 at 16 43 44

Example use in sapera-website

Screen Shot 2020-10-16 at 17 21 43

Technical

  • 馃敡 Add props and update onChange function
  • 馃敡 Add checkbox story to demo error message
  • 馃寛 Update colors in util
  • 馃寛 update spacing and alignment

Ticket

Related:

@a14m
Copy link
Contributor

a14m commented Oct 20, 2020

Done quick QA on the netlify deployed storybook https://5f8d454d4c3d0c5f820ffdaf--sapera-storybook.netlify.app/?path=/story/checkbox--default

looks working to me

@a14m a14m merged commit 434953d into master Oct 20, 2020
@a14m a14m deleted the feat/update-checkbox branch October 20, 2020 09:42
@cawo88
Copy link
Contributor

cawo88 commented Oct 20, 2020

I quickly QA in Chrome, Safari and Firefox and found one minor cosmetic issue. Please use a transparent background color inside the checkbox. Therefore it will use the same color as the body background color to allow more flexibility. See attached.

Screenshot 2020-10-20 at 11 51 56

@kaylarobertson3 kaylarobertson3 mentioned this pull request Oct 20, 2020
@kaylarobertson3
Copy link
Contributor Author

kaylarobertson3 commented Oct 20, 2020

@cawo88 Here is the follow-up PR fixing the blocking comment #41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants