Introduce Atomic Add to Cart Block #2705
Conversation
mikejolley
added
the
block-type: product elements
Add Notices to Single Product Block to catch add to cart ...Add Notices to Single Product Block to catch add to cart errors https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/92a9f28c8a01a73427f9b9d8ecf9f49e684c3c65/assets/js/atomic/blocks/product/add-to-cart/shared/context.js#L52-L63🚀 This comment was generated by the automations bot based on a
|
Update cart and checkout to use button component instead ...Update cart and checkout to use button component instead of cart-checkout/button. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/92a9f28c8a01a73427f9b9d8ecf9f49e684c3c65/assets/js/base/components/button/index.js#L16-L27🚀 This comment was generated by the automations bot based on a
|
|
Size Change: +7.71 kB (0%) Total Size: 1.58 MB
ℹ️ View Unchanged
|
mikejolley
force-pushed
the
add/cart-form-block
branch
from
92a9f28
to
22475ee
Compare
Update cart and checkout button component.Update cart and checkout button component. Cart and checkout should use base/components/button instead of cart-checkout/button. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/22475ee9c1dfb843764858b0c996dec497828639/assets/js/base/components/button/index.js#L16-L27🚀 This comment was generated by the automations bot based on a
|
The add to cart form may have several inner form elements...Introduce Validation Emitter for the Add to Cart Form The add to cart form may have several inner form elements which need to run validation andchange whether or not the form can be submitted. They may also need to show errors andvalidation notices. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/22475ee9c1dfb843764858b0c996dec497828639/assets/js/base/context/add-to-cart-form-context.js#L79-L94🚀 This comment was generated by the automations bot based on a
|
onChange should trigger when a form element changes, so f...Add Event Callbacks to the Add to Cart Form onChange should trigger when a form element changes, so for example, a variation picker could indicate that it's ready.onSuccess should trigger after a successful add to cart. This could be used to reset form elements, do a redirect, or show something to the user.onFail should trigger when adding to cart fails. Form elements might show extra notices or reset. A fallback might be to redirect to the core product page in case of incompatibilities. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/22475ee9c1dfb843764858b0c996dec497828639/assets/js/base/context/add-to-cart-form-context.js#L102-L125🚀 This comment was generated by the automations bot based on a
|
If the addToCart function within useStoreAddToCart fails,...Surface add to cart errors in the single product block If the addToCart function within useStoreAddToCart fails, a notice should be shown on the product page. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/22475ee9c1dfb843764858b0c996dec497828639/assets/js/base/context/add-to-cart-form-context.js#L94-L112🚀 This comment was generated by the automations bot based on a
|
Surface add to cart errors in the single product block.Surface add to cart errors in the single product block. If the addToCart function within useStoreAddToCart fails, a notice should be shown on the product page. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/93ed273207aa022f469d8423e4abf83433ba4777/assets/js/base/context/add-to-cart-form-context.js#L94-L112🚀 This comment was generated by the automations bot based on a
|
Add Event Callbacks to the Add to Cart Form.Add Event Callbacks to the Add to Cart Form.
https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/93ed273207aa022f469d8423e4abf83433ba4777/assets/js/base/context/add-to-cart-form-context.js#L102-L125🚀 This comment was generated by the automations bot based on a
|
| const VariationPicker = () => { | ||
| return ( | ||
| <Placeholder className="wc-block-components-product-add-to-cart-variation-picker"> | ||
| This is a placeholder for the variation picker form element. |
There was a problem hiding this comment.
This form element will be built in a follow up.
| disabled, | ||
| onClick, | ||
| } ) => { | ||
| const [ wasClicked, setWasClicked ] = useState( false ); |
There was a problem hiding this comment.
This is show we only show qty in cart after using the add to cart button.
There was a problem hiding this comment.
Curious - what's the problem with showing the amount in cart on page refresh? I guess that's a design question, but feels appropriate to me. If you've added the product to cart, the button consistently shows you this.
There was a problem hiding this comment.
I wanted to make sure it was shown as an "Add to Cart" button on first load like the mocks, rather than a button which says "x in cart". I think design will need another pass over all the blocks when we have them all built :)
|
|
||
| if ( ! product || ! product.categories ) { | ||
| if ( isEmpty( product ) || isEmpty( product.categories ) ) { |
There was a problem hiding this comment.
Stops {} products from being used.
| * @typedef {import('@woocommerce/type-defs/contexts').AddToCartFormContext} AddToCartFormContext | ||
| */ | ||
|
|
||
| const AddToCartFormContext = createContext( { |
There was a problem hiding this comment.
I've listed events and emitters I think we'll need but I will most likely need some help (in the follow up) making these work - it should be similar to the checkout extensibility work.
| * | ||
| * @param {number} quantityInCart Quantity of the item in the cart. | ||
| */ | ||
| export const useTriggerFragmentRefresh = ( quantityInCart ) => { |
There was a problem hiding this comment.
Moved this out to a hook to make it more reusable.
mikejolley
requested review from
haszari and
Aljullu
and removed request for
a team
| * | ||
| * @todo Update cart and checkout button component. Cart and checkout should use base/components/button instead of cart-checkout/button. | ||
| */ | ||
| const Button = ( { className, showSpinner = false, children, ...props } ) => { |
There was a problem hiding this comment.
This is a duplicate of the cart/checkout button - I logged a todo to migrate the cart/checkout to this one instead.
There was a problem hiding this comment.
While we're looking at this, is the docblock description accurate? It looks like the button vs a might come from Gutenberg button. This component seems to be focused on providing various classes + markup for our styles, and providing spinner.
There was a problem hiding this comment.
It does indeed, however since this implements that button I think it's clearer to state that this is what will happen if used.
| continue; | ||
| } | ||
|
|
||
| if ( $term->term_id === $default_category ) { |
There was a problem hiding this comment.
Found a bug whilst testing where the 'uncategorized' category was being listed. This prevents that happening.
There was a problem hiding this comment.
This is all looking good and testing well. I tested in Chrome only with the dev build (was exploring component tree in dev tools).
I tested all product types, didn't see any problems. Also tested external product type - I think this is fully functional in this PR? Worked correctly - button shows custom text and links out to external url.
Added a bunch of inline comments for minor stuff and to improve my understanding, none of this is blocking 👍
|
|
||
| return ( | ||
| <> | ||
| <VariationPicker /> |
There was a problem hiding this comment.
Looks like this VariableProductForm is very similar to SimpleProductForm - is it worth combining them and adding a prop for showing variation picker? Not a biggie.
There was a problem hiding this comment.
I think whilst the requirements are not completely clear (some of this may change as we built in extensibility) we should keep them separate and evaluate if they should merge once the picker is merged.
| /** | ||
| * Quantity Input Component. | ||
| */ | ||
| const QuantityInput = ( { disabled, min, max, value, onChange } ) => { |
There was a problem hiding this comment.
I thought this might use QuantitySelector from cart. I guess that's a design consideration, for now we want to be consistent with current add-to-cart button?
There was a problem hiding this comment.
Correct, the designs use a plain input.
| .wc-block-components-product-add-to-cart-quantity { | ||
| margin: 0 1em em($gap-small) 0; | ||
| width: 5em; | ||
| padding: 0.618em; |
There was a problem hiding this comment.
Can we get this padding value from a constant? (Minor!)
There was a problem hiding this comment.
👍 I got this value from the button styles so they had equal height. There is an issue to handle styles in a follow up across all atomic blocks: #2706
| * | ||
| * @todo Update cart and checkout button component. Cart and checkout should use base/components/button instead of cart-checkout/button. | ||
| */ | ||
| const Button = ( { className, showSpinner = false, children, ...props } ) => { |
There was a problem hiding this comment.
While we're looking at this, is the docblock description accurate? It looks like the button vs a might come from Gutenberg button. This component seems to be focused on providing various classes + markup for our styles, and providing spinner.
| component: Button, | ||
| }; | ||
|
|
||
| export const Default = () => ( |
There was a problem hiding this comment.
Great to see you added stories for this! I'll give them a test too :)
| disabled, | ||
| onClick, | ||
| } ) => { | ||
| const [ wasClicked, setWasClicked ] = useState( false ); |
There was a problem hiding this comment.
Curious - what's the problem with showing the amount in cart on page refresh? I guess that's a design question, but feels appropriate to me. If you've added the product to cart, the button consistently shows you this.
|
|
||
| Block.propTypes = { | ||
| className: PropTypes.string, | ||
| product: PropTypes.object, |
There was a problem hiding this comment.
Minor - should we declare our expectations for product shape? e.g. { id, type, add_to_cart: { ?url, ?text } }
There was a problem hiding this comment.
I didn't do this because product can be null and contains lots more props that those. There doesn't appear to be an existing pattern for doing this for product objects.
There was a problem hiding this comment.
I think you can do a union-style null or shapeOf but probably limited value doing this.
| disabled={ disabled } | ||
| showSpinner={ loading } | ||
| onClick={ ( e ) => { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
Why is preventDefault needed here?
There was a problem hiding this comment.
Beats me, Ill remove it.
| import { useAddToCartFormContext } from '@woocommerce/base-context'; | ||
|
|
||
| /** | ||
| * Add to Cart Form Qty + Button Block Component. |
There was a problem hiding this comment.
Is this comment accurate? This looks like just the CTA/button component, the quantity picker is elsewhere.
| }, | ||
| } = product; | ||
|
|
||
| if ( ( showFormElements || ! hasOptions ) && isPurchasable ) { |
There was a problem hiding this comment.
I'm a bit confused on why this is conditional on showFormElements || ! hasOptions. I'm guessing this is appropriate, can you clarify so I understand? Maybe add a comment if you think that's useful.
showFormElements - does this component need to be aware, or could the higher-level component (Block) simply not render AddToCartButton when form elements are toggled off?
hasOptions - when a product doesn't have options, why would that force the button to render?
There was a problem hiding this comment.
Added a comment.
// If we are showing form elements, OR if the product has no additional form options, we can show
// a functional direct add to cart button, provided that the product is purchasable.
// No link is required to the full form under these circumstances.
|
Merging this in; it's behind a feature flag! |
Adds an experimental/in-progress Add To Cart Atomic Block. This is a wrapper for the add to cart form button, qty picker, and any form elements that other product types require.
I've logged some inline todo's for context related follow ups (these are to support inner form elements and extensibility). There are also follow ups for:
So whilst the basic functionality is testable, this is not a final product :)
Closes #2676
How to test