This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Product Gallery > Next/Previous Buttons block: Add support to Interactivity API #10938
Conversation
This file contains 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
…ck-with-clicked-page-from-the-pager-block
…ck-with-clicked-page-from-the-pager-block
thealexandrelara
added
type: enhancement
woocommercebot
requested review from
a team and
danieldudzic
and removed request for
a team
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +63 B (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
gigitux
suggested changes
Sep 14, 2023
| }, | ||
| }, | ||
| actions: { | ||
| woocommerce: { | ||
| handleClick: ( { context }: Store ) => { | ||
| context.woocommerce.selectedImage = context.woocommerce.imageId; | ||
| }, | ||
| onNextImageButtonClick: ( store: Store ) => { |
There was a problem hiding this comment.
Usually, the handler function should have the prefix handle. What do you think?
Suggested change
| onNextImageButtonClick: ( store: Store ) => { | |
| handleNextImageButtonClick: ( store: Store ) => { |
Also, we should decide if we want a nested structure or not (#10823 (comment)). Mmm, I don't have a strong preference, but it makes sense if we have a common approach.
There was a problem hiding this comment.
I just changed it from onNext/onPrevious to handleNext/handlePrevious so we establish a common pattern to follow
Regarding the nested structure, I believe that since the store is for the entire block (the Product Gallery block), we can leave it flat. However, if there are actions, selectors, or state that should only be used in the context of one of the inner blocks, we can nest them (this way, we make it clear that they should only be used within that specific inner block).
|
Ops, I just noticed that a lot of code in this PR is in #10736. Please use ZenHub to specify some dependencies between PRs or/and write it in the description PR! 👍 |
…ked-page-from-the-pager-block' into feat/10631-replace-image-in-large-image-block-when-clicking-next-or-previous-button
My bad 🤦 , I connected the issue but forgot to add the dependency, thank you for pointing this out and for reviewing the PR! |
…ck-with-clicked-page-from-the-pager-block
…ck-when-clicking-next-or-previous-button
…ck-when-clicking-next-or-previous-button
…ck-when-clicking-next-or-previous-button
13 tasks
gigitux
suggested changes
Oct 4, 2023
…ck-when-clicking-next-or-previous-button
…ck-when-clicking-next-or-previous-button
13 tasks
gigitux
suggested changes
Oct 5, 2023
Thank you for pointing this out, I just fixed this in here: e8a9edb |
gigitux
approved these changes
Oct 5, 2023
|
I noticed that the PHP linter job step fails. Could you check why, @thealexandrelara? |
…ck-when-clicking-next-or-previous-button
Delete unnecessary file that is causing php cs error
thealexandrelara
deleted the
feat/10631-replace-image-in-large-image-block-when-clicking-next-or-previous-button
branch
Yes, for some reason a random empty php file was added to the PR, not sure if it is related to the issue that Github is currently facing with PRs, but after removing this file, it worked |
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.
What
This PR adds support to the Interactivity API on the Next/Previous buttons block, allowing shoppers to navigate between the gallery images without having to reload the page.
Fixes #10631
Why
This is part of our effort to implement the new Product Gallery block.
Testing Instructions
Please consider any edge cases this change may have, and also other areas of the product this may impact.
Screenshots or screencast
CleanShot.2023-09-13.at.17.15.42.mp4
WooCommerce Visibility
Required: