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

Remove data attributes from being saved in html of All products block #10032

Merged
merged 13 commits into from
Sep 25, 2023
Merged

Remove data attributes from being saved in html of All products block #10032

merged 13 commits into from
Sep 25, 2023

Conversation

hritikchaudhary
Copy link
Contributor

Fixes #9961

Changes in the PR:

Removed propTypes definitions from Reviews By Category.

Testing

Automated Tests

  • Changes in this PR are covered by Automated Tests.
    • Unit tests
    • E2E tests

User Facing Testing

  • Do not include in the Testing Notes

WooCommerce Visibility

  • WooCommerce Core
  • Feature plugin
  • Experimental

@hritikchaudhary hritikchaudhary mentioned this pull request Jun 30, 2023
Closed
@roykho roykho self-requested a review July 3, 2023 12:49
@roykho roykho added the block: all products Issues related to the all products block. label Jul 3, 2023
@roykho
Copy link
Member

roykho commented Jul 3, 2023

Hi @hritikchaudhary - thanks for submitting this however you will need to deprecate here as we discussed on #9961 otherwise users will see block validation errors.

@github-actions
Copy link
Contributor

This PR has been marked as stale because it has not seen any activity within the past 7 days. Our team uses this tool to help surface pull requests that have slipped through review.

If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Jul 11, 2023
@github-actions github-actions bot removed the status: stale Stale issues and PRs have had no updates for 60 days. label Jul 12, 2023
@github-actions
Copy link
Contributor

This PR has been marked as stale because it has not seen any activity within the past 7 days. Our team uses this tool to help surface pull requests that have slipped through review.

If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Jul 20, 2023
@roykho
Copy link
Member

roykho commented Jul 20, 2023

Hi @hritikchaudhary - any updates on this? If you don't have time to look more into this, I am more than happy to take this over.

@github-actions github-actions bot removed the status: stale Stale issues and PRs have had no updates for 60 days. label Jul 21, 2023
@hritikchaudhary
Copy link
Contributor Author

Hi @roykho I'm really sorry, this one slipped out of my radar...
I saw there is already a deprecated.js file in the same directory, do I have to use that or create a new one?

@roykho
Copy link
Member

roykho commented Jul 24, 2023

Hi @hritikchaudhary - yes you can use the same file and just create a v2 object.

@hritikchaudhary
Copy link
Contributor Author

Ok, Thanks for the clarification.

@hritikchaudhary
Copy link
Contributor Author

Hi @roykho, I have added old save function in deprecated.js, Not sure how to modify save.js so it indicates that save function is deprecated.
Apologies, I'm still learning react, I am more experienced with Angular 😬

roykho
roykho suggested changes Aug 1, 2023
View reviewed changes
Copy link
Member

@roykho roykho 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 pushing up updates. I've left some comments.

assets/js/blocks/products/all-products/save.js Outdated Show resolved Hide resolved
assets/js/blocks/products/all-products/deprecated.js Show resolved Hide resolved
assets/js/blocks/products/all-products/deprecated.js Outdated Show resolved Hide resolved
@hritikchaudhary hritikchaudhary requested a review from roykho August 1, 2023 15:08
@roykho roykho mentioned this pull request Aug 1, 2023
Closed
roykho
roykho suggested changes Aug 1, 2023
View reviewed changes
Copy link
Member

@roykho roykho left a comment

Choose a reason for hiding this comment

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

Hi @hritikchaudhary - some e2e tests are not passing as seen here could you please take a look at those and fix/update the tests if needed so that they will pass?

@hritikchaudhary
Copy link
Contributor Author

Hi @roykho I'm facing some discrepancies in e2e test result, but was able to pin point one test case that was common between my local and workflow result.
[blockTheme] › mini-cart/mini-cart.block_theme.spec.ts:122:7 › woocommerce/mini-cart Block › with All products Block › should open the filled cart drawer
Can you please provide some guidance related to how to fix this e2e testcase... or just point where I can look for help and debug, that'd be very helpful.
Thanks!

@roykho roykho added the skip-changelog PRs that you don't want to appear in the changelog. label Sep 25, 2023
@roykho roykho merged commit 829fdec into woocommerce:trunk Sep 25, 2023
tarhi-saad added a commit that referenced this pull request Sep 27, 2023
@tarhi-saad
Revert #10032
54928b7
tarhi-saad added a commit that referenced this pull request Sep 27, 2023
@tarhi-saad
Reapply changes from #10032
b935c56
tarhi-saad added a commit that referenced this pull request Sep 28, 2023
@tarhi-saad
Revert #10032
a9528ff
tarhi-saad added a commit that referenced this pull request Sep 28, 2023
@tarhi-saad
Reapply changes from #10032
b7e88c4
tarhi-saad added a commit that referenced this pull request Sep 28, 2023
@tarhi-saad
Revert #10032
45a4fa3
tarhi-saad added a commit that referenced this pull request Sep 28, 2023
@tarhi-saad
Reapply changes from #10032
6fc5111
tarhi-saad added a commit that referenced this pull request Sep 28, 2023
@tarhi-saad
Fix failing Playwright E2E tests (#11055)
8b4d865 
* Revert #11029 to fix failing E2E tests

* Revert #10032

* Reapply changes from #10032

* Skip failing E2E test

* Skip Price Filter Block failing tests

* Apply back changes from #11029

* Fix Checkout template E2E tests
kmanijak added a commit that referenced this pull request Oct 16, 2023
@kmanijak
Revert #10032 so All Products renders in the frontend
9f6da39
@kmanijak kmanijak mentioned this pull request Oct 16, 2023
Merged
13 tasks
kmanijak added a commit that referenced this pull request Oct 16, 2023
@kmanijak
Revert #10032 so All Products renders in the frontend (#11263)
3e4b656
roykho pushed a commit that referenced this pull request Oct 16, 2023
@kmanijak @roykho
Revert #10032 so All Products renders in the frontend (#11263)
97bf32f
roykho added a commit that referenced this pull request Oct 17, 2023
@github-actions @roykho @kmanijak
Release: 11.3.1 (#11272)
d543901 
* Empty commit for release pull request

* Update to version 11.3.1

* Revert #10032 so All Products renders in the frontend (#11263)

* Add testing instructions

* Fix linting errors in testing instructions for 1130

* Update testing instructions

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: roykho <roykho77@gmail.com>
Co-authored-by: Karol Manijak <20098064+kmanijak@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@roykho roykho roykho requested changes

Assignees
No one assigned
Labels
block: all products Issues related to the all products block. skip-changelog PRs that you don't want to appear in the changelog. type: community contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

All Products Block saving data attributes in HTML
3 participants
@hritikchaudhary @roykho @dinhtungdu