This repository was archived by the owner on Feb 23, 2024. It is now read-only.
ProductButton: Add animation #10351
Merged
ProductButton: Add animation #10351
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gigitux
commented
Jul 26, 2023
| return ref.textContent; | ||
| } | ||
|
|
||
| ref.textContent = ''; |
There was a problem hiding this comment.
This is necessary otherwise, the returned string (line 97) is appended.
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/atomic/blocks/product-elements/button/frontend.tsx
assets/js/atomic/blocks/product-elements/rating-stars/index.tsx assets/js/atomic/blocks/product-elements/rating-stars/support.ts assets/js/blocks/classic-template/test/utils.ts assets/js/blocks/mini-cart/edit.tsx assets/js/blocks/mini-cart/index.tsx assets/js/blocks/mini-cart/mini-cart-contents/edit.tsx assets/js/blocks/product-gallery/edit.tsx assets/js/hocs/test/with-searched-products.js assets/js/interactivity/store.js assets/js/interactivity/utils.js |
e5ff92b to
1ec122b
Compare
|
Size Change: +878 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Aljullu
reviewed
Jul 26, 2023
Comment on lines
66
to
74
| span { | ||
|
|
||
| &.wc-block-scrollToUp { | ||
| animation: scrollToUp 0.1s linear 1 normal; | ||
| } | ||
| &.wc-block-scrollFromDown { | ||
| animation: scrollFromDown 0.1s linear 1 normal; | ||
| } | ||
| } |
There was a problem hiding this comment.
This should probably be wrapped with prefers-reduced-motion.
src/BlockTypes/ProductButton.php
Outdated
| ), | ||
| $span_button_directives = 'data-wc-text="selectors.woocommerce.addToCartText"'; | ||
|
|
||
| if ( WC()->cart->is_empty() ) { |
There was a problem hiding this comment.
This is causing me an error when saving posts. It looks like WC()->cart is null, so the is_empty() method doesn't exist. Can you reproduce as well?
|
Thanks, @Aljullu, I had a pair session with @luisherranz that explained my wrong approach. Thanks to his help, I refactored this code and implemented the animation when the quantity of the product is updated from the Mini Cart as well. |
Aljullu
reviewed
Jul 28, 2023
There was a problem hiding this comment.
As mentioned in the previous review, this is looking awesome, I'm very excited for this to be merged. 👏
I left some comments inline, but they are mostly about coding style, so feel free to disregard them if you don't agree.
Also, I noticed that using the woocommerce_add_to_cart_quantity filter doesn't work anymore. Steps to reproduce:
- In any PHP file, add this code:
// Function to set the quantity of items added to cart
function custom_add_to_cart_quantity( $quantity, $product_id ) {
return 10;
}
add_filter( 'woocommerce_add_to_cart_quantity', 'custom_add_to_cart_quantity', 10, 2 );- In a page with the Products block, add a product to the cart.
- Notice only one product is added, instead of
10as specified in the filter from step 1.
Can you reproduce as well?
| state, | ||
| context, | ||
| }: { | ||
| selecotrs: any; |
| hasCartLoaded: ( { state }: { state: State } ) => { | ||
| return state.woocommerce.cart !== undefined; | ||
| }, | ||
| numberOfItems: ( { |
There was a problem hiding this comment.
Given that this is a function, I think it would be great to start its name with a verb, something like getNumberofItems(). What do you think?
Comment on lines
178
to
191
| const isFreshLoad = | ||
| selectors.woocommerce.hasCartLoaded( store ) && | ||
| context.woocommerce.initialNumberOfItems !== | ||
| selectors.woocommerce.numberOfItems( store ) && | ||
| context.woocommerce.numberOfItems !== | ||
| selectors.woocommerce.numberOfItems( store ); | ||
|
|
||
| const isFromCart = | ||
| selectors.woocommerce.numberOfItemsPrev( store ) !== | ||
| undefined && | ||
| selectors.woocommerce.numberOfItemsPrev( store ) !== | ||
| selectors.woocommerce.numberOfItems( store ) && | ||
| selectors.woocommerce.numberOfItems( store ) !== | ||
| context.woocommerce.numberOfItems; |
There was a problem hiding this comment.
Nit: given that we are calling numberOfItems() several times, could we call it at the beginning of the function and save its value into a variable? Same with numberOfItemsPrev(). I feel it would make the code easier to read.
|
|
||
| return product?.quantity || 0; | ||
| }, | ||
| numberOfItemsPrev: ( { |
There was a problem hiding this comment.
I would change this one to start with get too, so it's clear it's a function.
e294c2e to
c86c4e8
Compare
28d935f to
c6acd18
Compare
Nice catch! I added the support to the filter with c6acd18 |
… and consolidate animation status
|
Hey Luigi, I think the number of variables storing I made a video to explain everything: 
https://www.loom.com/share/19478a6e951442f09f5902d3fb20c68e I still want to take a look at how to await the population of the store in the |
Thanks for your work and your great explanation! I forget entirely about the With 1333cc7 I re-added the |
|
Regarding the other improvements that you mentioned:
Should we continue to work on it, or can we start to merge this and work on them with follow-up issues/PRs? |
It works fine 🙂 
https://www.loom.com/share/3dde7d89d1c8496fa8c6d1b4a2652f57
Yeah, let's merge this PR now. I'd like to take a look at the need for React and |
|
Regarding the I removed the logic for the |
|
If you think that everything is ready, could you approve the PR? @luisherranz 🙇 |
luisherranz
approved these changes
Aug 2, 2023
gigitux
added a commit
that referenced
this pull request
Aug 3, 2023
…10338) * show notice when there is an error * Add initial implementation for store callbacks * Run `afterLoad` callbacks after `init` * Move cart state subscription to Product button * Remove cart-store from Interactivity API internals * Change callbacks with options and save only afterLoad callbacks * ProductButton: Add animation (#10351) * implement animation * improve logic * refactor logic * refactor code * address feedback about code style * add support for woocommerce_add_to_cart_quantity * Fix animation flickering * Introduce wp-effect, reduce the amount of numberOfItem variables to 2 and consolidate animation status * add support for added class * Remove unnecessary selector * Don't fetch cart if it was already fetched * remove added class --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> --------- Co-authored-by: Luigi <gigitux@gmail.com> Co-authored-by: Luis Herranz <luisherranz@gmail.com>
gigitux
added a commit
that referenced
this pull request
Aug 10, 2023
* Update Interactivity API JS files * Disable TS checks in the Interactivity API for now * Add new SSR files * Replace wp_ prefixes with wc_ ones * Replace wp- prefix with wc- * Replace guternberg_ prefix with woocommerce_ * Remove file comments from Gutenberg * Rename files with `wp` prefix * Fix code to load Interactivity API php files * Remove TODO comments * Replace @WordPress with @woocommerce * Update Webpack configuration * Fix directive prefix * Remove interactivity folder from tsconfig exclude * Add client-side navigation meta tag code * Remove unneeded blocks.php file * Fix store tag id * Register Interactivity API runtime script * Fix Interactivity API runtime registering * Remove all files related to directive processing in PHP * Move json_encode to Store's render method * WIP * WIP * WIP * WIP * Preserve previous context * Ignore Minicart block on client-side navigation * Refresh page on store updatRefresh page on store updatee * Refactor logic * Add console error when a path is missing * fix PHP lint error * WIP store * use store approach * update jest configuration * restore Mini Cart changes * move cart store subscription to interactivity package * move interactivity flag * format HTML * move addToCartText to the context * Load product-query stylesheet when rendering the Products block * update sideEffects array * fix catch * rename moreThanOneItem to isThereMoreThanOneItem * improve how scripts are enqueued * update default value for the filter woocommerce_blocks_enable_interactivity_api * Update assets/js/atomic/blocks/product-elements/button/block.json Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> * Update assets/js/interactivity/cart/cart-store.ts Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> * fix block.json * remove updateStore function * restore interactivity api changes * import cart store * show notice when there is an error * add logic to dequeue script on classic themes and block themes * imrpove logic about notice * Interactivity API: add `afterLoad` callbacks to `store()` function (#10338) * show notice when there is an error * Add initial implementation for store callbacks * Run `afterLoad` callbacks after `init` * Move cart state subscription to Product button * Remove cart-store from Interactivity API internals * Change callbacks with options and save only afterLoad callbacks * ProductButton: Add animation (#10351) * implement animation * improve logic * refactor logic * refactor code * address feedback about code style * add support for woocommerce_add_to_cart_quantity * Fix animation flickering * Introduce wp-effect, reduce the amount of numberOfItem variables to 2 and consolidate animation status * add support for added class * Remove unnecessary selector * Don't fetch cart if it was already fetched * remove added class --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> --------- Co-authored-by: Luigi <gigitux@gmail.com> Co-authored-by: Luis Herranz <luisherranz@gmail.com> * update deepsignal * remove added class * update deepsignal * Interactivity API and Product Button: Add E2E tests (#10036) * Add FrontendUtils class * fix conflicts * use locator * restore click usage * Product Button: Add E2E test * fix util * fix E2E tests * remove comment * Add E2E test to ensure that woocommerce_product_add_to_cart_text works * update sideEffects array * add zip and unzip as package * fix wp-env configuration * fix E2E test * add report * try now * try now * try now * fix E2E test * E2E: Add documentation for testing actions and filters. Fixes #10135 (#10206) * update description * fix label * rename files * make requestUtils private * remove page.goto * use toHaveCount * use productsToDisplay variable * fix E2E tests * rename class utils --------- Co-authored-by: Daniel Dudzic <daniel.dudzic@automattic.com> --------- Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: Luis Herranz <luisherranz@gmail.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Daniel Dudzic <daniel.dudzic@automattic.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the animation discussed into https://github.com/woocommerce/woocommerce-blocks/discussions/10153#discussioncomment-6475505.
Screen.Capture.on.2023-07-26.at.16-22-00.mp4
Screen.Capture.on.2023-07-28.at.12-06-26.mp4
Testing
User Facing Testing