Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Add storybook demo for CheckboxControl #3030

Merged
merged 2 commits into from
Dec 10, 2020
Merged

Add storybook demo for CheckboxControl #3030

merged 2 commits into from
Dec 10, 2020

Conversation

haszari
Copy link
Member

@haszari haszari commented Aug 20, 2020

Part of #3031

Accessibility

Screenshots

How to test the changes in this Pull Request:

  1. Execute npm run storybook
  2. Visit the storybook URL (for me it is http://localhost:6006/) and then go to @base-components > CheckboxControl > CheckboxControl
  3. Test the component to ensure the box becomes checked when clicking the label or checkbox.
  4. Test with screen reader on and by using only the keyboard.
  5. Check the accessibility tab of Storybook and verify that the contrast of the component is ok.

@nerrad nerrad changed the base branch from main to trunk September 26, 2020 17:30
@nerrad nerrad added tools Used for work on build or release tools. type: cooldown Things that are queued for a cooldown period (assists with planning). labels Oct 1, 2020
@opr opr self-assigned this Dec 4, 2020
@opr opr changed the title add storybook demo for CheckboxControl Add storybook demo for CheckboxControl Dec 9, 2020
@opr opr marked this pull request as ready for review December 9, 2020 11:50
@opr opr requested a review from a team as a code owner December 9, 2020 11:50
@opr opr requested review from Aljullu and removed request for a team December 9, 2020 11:50
@Aljullu
Copy link
Contributor

Aljullu commented Dec 10, 2020

@opr is this ready for review? I see the diff is quite big (+28k, -19k) and I'm not sure what files need to be reviewed, any clues? I think maybe undoing the merge and instead rebasing this branch with trunk would simplify the diff? Otherwise, what about cherry picking the relevant commits into a new branch based on trunk? I'm a bit wary of merging a PR as big as this one, specially considering github-actions reports some assets increase.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2020

Size Change: 0 B

Total Size: 1.17 MB

ℹ️ View Unchanged
Filename Size Change
build/active-filters-frontend.js 8.9 kB 0 B
build/active-filters.js 8.92 kB 0 B
build/all-products-frontend.js 34.8 kB 0 B
build/all-products.js 36.2 kB 0 B
build/all-reviews.js 9.74 kB 0 B
build/atomic-block-components/add-to-cart--atomic-block-components/button.js 3.29 kB 0 B
build/atomic-block-components/add-to-cart--atomic-block-components/image--atomic-block-components/title.js 335 B 0 B
build/atomic-block-components/add-to-cart-frontend.js 9.02 kB 0 B
build/atomic-block-components/add-to-cart.js 7.53 kB 0 B
build/atomic-block-components/button-frontend.js 2.31 kB 0 B
build/atomic-block-components/button.js 839 B 0 B
build/atomic-block-components/category-list-frontend.js 470 B 0 B
build/atomic-block-components/category-list.js 476 B 0 B
build/atomic-block-components/image-frontend.js 1.68 kB 0 B
build/atomic-block-components/image.js 1.13 kB 0 B
build/atomic-block-components/price-frontend.js 2.29 kB 0 B
build/atomic-block-components/price.js 2.31 kB 0 B
build/atomic-block-components/rating-frontend.js 524 B 0 B
build/atomic-block-components/rating.js 530 B 0 B
build/atomic-block-components/sale-badge-frontend.js 858 B 0 B
build/atomic-block-components/sale-badge.js 862 B 0 B
build/atomic-block-components/sku-frontend.js 389 B 0 B
build/atomic-block-components/sku.js 394 B 0 B
build/atomic-block-components/stock-indicator-frontend.js 569 B 0 B
build/atomic-block-components/stock-indicator.js 573 B 0 B
build/atomic-block-components/summary-frontend.js 916 B 0 B
build/atomic-block-components/summary.js 926 B 0 B
build/atomic-block-components/tag-list-frontend.js 466 B 0 B
build/atomic-block-components/tag-list.js 470 B 0 B
build/atomic-block-components/title-frontend.js 1.21 kB 0 B
build/atomic-block-components/title.js 1.05 kB 0 B
build/attribute-filter-frontend.js 18.3 kB 0 B
build/attribute-filter.js 12.5 kB 0 B
build/blocks.js 3.52 kB 0 B
build/cart-frontend.js 77.2 kB 0 B
build/cart.js 40.6 kB 0 B
build/checkout-frontend.js 92.4 kB 0 B
build/checkout.js 43 kB 0 B
build/editor-rtl.css 14.6 kB 0 B
build/editor.css 14.6 kB 0 B
build/featured-category.js 7.74 kB 0 B
build/featured-product.js 9.97 kB 0 B
build/handpicked-products.js 7.37 kB 0 B
build/price-filter-frontend.js 14.9 kB 0 B
build/price-filter.js 10.3 kB 0 B
build/product-best-sellers.js 7.44 kB 0 B
build/product-categories.js 3.22 kB 0 B
build/product-category.js 8.39 kB 0 B
build/product-new.js 7.61 kB 0 B
build/product-on-sale.js 8 kB 0 B
build/product-search.js 3.54 kB 0 B
build/product-tag.js 6.56 kB 0 B
build/product-top-rated.js 7.58 kB 0 B
build/products-by-attribute.js 8.36 kB 0 B
build/reviews-by-category.js 11.8 kB 0 B
build/reviews-by-product.js 13.3 kB 0 B
build/reviews-frontend.js 9.38 kB 0 B
build/single-product-frontend.js 37.9 kB 0 B
build/single-product.js 10.1 kB 0 B
build/style-rtl.css 18.4 kB 0 B
build/style.css 18.4 kB 0 B
build/vendors--atomic-block-components/price-frontend.js 5.65 kB 0 B
build/vendors-style-rtl.css 1.05 kB 0 B
build/vendors-style.css 1.05 kB 0 B
build/vendors.js 439 kB 0 B
build/wc-blocks-data.js 6.89 kB 0 B
build/wc-blocks-middleware.js 931 B 0 B
build/wc-blocks-registry.js 2.39 kB 0 B
build/wc-payment-method-bacs.js 775 B 0 B
build/wc-payment-method-cheque.js 771 B 0 B
build/wc-payment-method-cod.js 866 B 0 B
build/wc-payment-method-paypal.js 813 B 0 B
build/wc-payment-method-stripe.js 12.1 kB 0 B
build/wc-settings.js 2.59 kB 0 B
build/wc-shared-context.js 1.53 kB 0 B
build/wc-shared-hocs.js 1.66 kB 0 B

compressed-size-action

Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

Thanks for updating the PR, @opr! The story works great and code looks good. I just added a minor comment about naming the component, but not a blocker.

/**
* Internal dependencies
*/
import Component from '../';
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like in other stories we are importing the components with their name (in this case, it would be CheckboxControl), and the story has a generic name (in some that I checked, it was named Default). Do you think we should keep it consistent here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I agree! I'll make this change

@opr opr merged commit 83da31e into trunk Dec 10, 2020
@opr opr deleted the add/checkbox-story branch December 10, 2020 10:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tools Used for work on build or release tools. type: cooldown Things that are queued for a cooldown period (assists with planning).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants