Skip to content

Update disable drag and drop within images block #38045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
louwie17 merged 4 commits into from
May 5, 2023
Merged

Update disable drag and drop within images block #38045

louwie17 merged 4 commits into from
May 5, 2023

Conversation

@louwie17
Copy link
Contributor

@louwie17 louwie17 commented Apr 28, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

This adds support to ImageGallery component to disable dragging, by the new allowDragging prop (defaults to true).
As a second change this disables dragging within the product images block.

Closes #37955

Screenshot 2023-04-28 at 3 51 51 PM

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Build and load this branch and enable the new-product-management-experience feature flag ( the non-block editor )
  2. Go to Products > Add New and scroll down to images and add some images.
  3. Now re-arrange them by dragging the images around, this should still work as expected.
  4. Now enable the block based editor, with the product-block-editor feature flag
  5. Go to Products > Add New and scroll down to images and add some images.
  6. Try dragging images around, this should not be possible, the drag handle should also not be visible within the image toolbar.
  7. Click on the arrows within the image toolbar, this should still move images around successfully.

@louwie17 louwie17 requested a review from a team April 28, 2023 13:52
@louwie17 louwie17 changed the title Update= disable drag and drop within images block Update disable drag and drop within images block Apr 28, 2023
@github-actions github-actions bot added the package: @woocommerce/components issues related to @woocommerce/components label Apr 28, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023 •
edited
Loading

Hi @joelclimbsthings,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023 •
edited
Loading

Test Results Summary

Commit SHA: 3f424fe

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests26700202691m 16s
E2E Tests1870010019714m 29s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@joelclimbsthings joelclimbsthings requested review from joelclimbsthings and removed request for a team May 2, 2023 21:38
joelclimbsthings
joelclimbsthings suggested changes May 2, 2023
View reviewed changes
Copy link
Contributor

@joelclimbsthings joelclimbsthings left a comment

Choose a reason for hiding this comment

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

Nice work @louwie17 ! The code looks good at first glance, although I'm encountering an error. If you:

  • Replace an image
  • Immediately click the arrow to attempt to move that image after it's replaced

I'd guess that the issue actually lies within the block itself, and was probably preexisting to this PR. Let me know if you'd rather address in a follow-up.

image

@louwie17 louwie17 force-pushed the update/37955_disable_drag_and_drop_images_block branch from fb3c3b2 to 6739f4a Compare May 4, 2023 11:40
@louwie17 louwie17 requested a review from joelclimbsthings May 4, 2023 11:40
@louwie17
Copy link
Contributor Author

louwie17 commented May 4, 2023

Thanks for the review @joelclimbsthings, it turned out to be an issue with the onReplace method, where its changes were not being reflected as we were altering the same array.
I assume this happened because of some logic within React or the data store that check if the array reference changed, and only updates the store if it changed. Directly altering the array location of a single item doesn't change this.
I fixed it in this commit: 6739f4a where I mapped it to a new array.

Should be ready for a re-review.

joelclimbsthings
joelclimbsthings previously approved these changes May 4, 2023
View reviewed changes
Copy link
Contributor

@joelclimbsthings joelclimbsthings left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice job @louwie17 ! Working well 🚢

Edit: Had to push a quick fix for a linting error in 3f424fe. Wanted to make sure it was mergable when you return.

@louwie17 louwie17 merged commit ce07458 into trunk May 5, 2023
@louwie17 louwie17 deleted the update/37955_disable_drag_and_drop_images_block branch May 5, 2023 07:45
@github-actions github-actions bot added this to the 7.8.0 milestone May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@joelclimbsthings joelclimbsthings joelclimbsthings approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@louwie17 louwie17

Labels

package: @woocommerce/components issues related to @woocommerce/components

Projects

None yet

Milestone

7.8.0

Development

Successfully merging this pull request may close these issues.

Product Block Editor: Form is broken after drag and dropping images within Images block

3 participants

@louwie17 @joelclimbsthings