This repository was archived by the owner on Feb 23, 2024. It is now read-only.
Migrate Featured Items Blocks to TypeScript #6439
Merged
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
Contributor
|
Size Change: +70 B (0%) Total Size: 862 kB
ℹ️ View Unchanged
|
Refactored some composed components into one inclusive HOC, called `withUpdateButtonAttributes`.
sunyatasattva
added
type: refactor
frontdevde
reviewed
May 27, 2022
Contributor
There was a problem hiding this comment.
When trying to follow the testing instructions and testing the Featured Product block, I immediately get an error in the editor upon block insertion. I double-checked and on trunk, appears to be working fine.
| trunk | branch |
|---|---|
 |
 |
The error I'm seeing on this branch (and not on trunk):
frontdevde
reviewed
May 27, 2022
frontdevde
reviewed
May 27, 2022
frontdevde
reviewed
May 27, 2022
frontdevde
reviewed
May 27, 2022
…/6237-6238-featured-items-typescript
The HOC `withTransformSingleSelectToMultipleSelect` allows for the wrong type to be passed to its wrapped component: if a single element is passed, that is wrapped into an array. This didn't play well with TypeScript which obviously complains about the wrong type being passed to the component. So we now allow for empty arrays to be passed and parsed as "nil".
Contributor
Convert `ProductControl` to TypeScript eslint-disable-nex...Convert `ProductControl` to TypeScript eslint-disable-next-line @typescript-eslint/ban-ts-comment @ts-ignore
woocommerce-blocks/assets/js/blocks/featured-items/with-edit-mode.tsx Lines 107 to 123 in f3bc88a 🚀 This comment was generated by the automations bot based on a
|
Contributor
Convert `ProductCategoryControl` to TypeScript eslint-dis...Convert `ProductCategoryControl` to TypeScript eslint-disable-next-line @typescript-eslint/ban-ts-comment @ts-ignore
woocommerce-blocks/assets/js/blocks/featured-items/with-edit-mode.tsx Lines 78 to 91 in f3bc88a 🚀 This comment was generated by the automations bot based on a
|
Contributor
Author
|
@frontdevde @albarin Feedback addressed! |
Contributor
|
I'm still seeing the same block error (see above) when I'm trying to insert the Featured Product block on this branch (not on trunk). Can you reproduce that issue? |
Contributor
Convert `ProductCategoryControl` to TypeScript eslint-dis...Convert `ProductCategoryControl` to TypeScript eslint-disable-next-line @typescript-eslint/ban-ts-comment @ts-ignore
woocommerce-blocks/assets/js/blocks/featured-items/with-edit-mode.tsx Lines 78 to 91 in 5d280cf 🚀 This comment was generated by the automations bot based on a
|
Contributor
Convert `ProductControl` to TypeScript eslint-disable-nex...Convert `ProductControl` to TypeScript eslint-disable-next-line @typescript-eslint/ban-ts-comment @ts-ignore
woocommerce-blocks/assets/js/blocks/featured-items/with-edit-mode.tsx Lines 107 to 123 in 5d280cf 🚀 This comment was generated by the automations bot based on a
|
frontdevde
approved these changes
May 30, 2022
Contributor
There was a problem hiding this comment.
Thank you for this valiant effort and for addressing the feedback!
When retesting with the most recent changes functionally everything appears to be working as expected. Let's get this in as the improvement over the status quo it is and iterate on anything else from there.
This was referenced May 30, 2022
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 migrates the
FeaturedItemblocks to TypeScript. I tried to keep the runtime changes at a bare minimum. A few very minor of them were required. The biggest one was the refactoring of two pieces of code that were once again duplicates and also a bit hard to reason with (especially with TypeScript). These were refactored intowith-update-button-attributes.tsx, wherewithSelectwas also updated to use the more modernuseSelecthook approach.All the rest is adding the most type safety I could muster, and working around some upstream type limitations.
Fixes #6237, #6238
Testing
Automated Tests
User Facing Testing
Only regression testing is needed for this PR. The changes introduced in this PR should be compared with a version without those changes and verified that no behavior has changed.
WooCommerce Visibility