Open file picker by clicking card #35358

octaedro · 2022-10-27T16:02:26Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR adds the code to allow users to open the file picker by clicking anywhere inside the product gallery card.

Closes #35183.

How to test the changes in this Pull Request:

Check out this branch and go to Products > Add New (MVP)
Click the Drag and drop area (not the Choose images button).
A file picker should be visible. Pick an image.
The selected image should be visible now in the gallery.
The tracks event wcadmin_product_images_add_via_file_upload_area should be recorded
Continue with the product creation process. Confirm that it has been created correctly.

Storybook
Check that the storybook for the component FileUploadArea is working as expected.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

github-actions · 2022-10-27T16:28:00Z

Test Results Summary

Commit SHA: 43c7366

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	1m 19s
E2E Tests	186	0	0	6	0	192	13m 56s

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.

octaedro · 2022-11-02T12:58:21Z

packages/js/components/src/file-upload-area/README.md

@@ -0,0 +1,29 @@
+# FileUploadArea


I planned to use the FormFileUpload component, but it renders everything inside a button. Since we needed to add another button inside the button, I leaned towards creating our own component to have a clickable area to upload files.

joshuatf

Starting to review this, but just have a general question on how this should work: should this open the file system's native file picker or the WP media modal to select files?

It's a tiny bit ambiguous to me based on discussion right now, but I would expect the media modal would be opened given that's what happens when clicking the button.

cc @jarekmorawski

octaedro · 2022-11-02T18:30:18Z

Thank you @joshuatf,

how this should work: should this open the file system's native file picker or the WP media modal to select files?

From what I understood, the button Choose images will open the media modal, and clicking the drag and drop area should open the file picker.

Maybe @jarekmorawski could confirm whether that's correct or not.

jarekmorawski · 2022-11-02T18:37:35Z

From what I understood, the button Choose images will open the media modal, and clicking the drag and drop area should open the file picker.

That's correct! That's why the copy says Drag images here or click to upload. Upload = select from your device, Choose = select from the library.

packages/js/components/src/media-uploader/README.md

joshuatf

Very nice work, Fernando! Testing well for me in the product screen. Left a couple of minor comments.

I think we may also need to update the "Mock media upload" example in Storybook as it's broken without the render function currently:

plugins/woocommerce-admin/client/products/sections/images-section.tsx

joshuatf · 2022-11-04T21:27:23Z

plugins/woocommerce-admin/client/products/sections/images-section.tsx

-											] );
-										}
+								<FormFileUpload
+									accept="image/*"


I think this might need to be updated to account for the allowedMediaTypes prop.

Good idea! I changed that in the commit aae35d0

octaedro · 2022-11-16T11:06:37Z

Hey @joshuatf,

Thank you for reviewing this PR.

I think we may also need to update the "Mock media upload" example in Storybook as it's broken without the render function currently:

That error doesn't look related to this PR. It didn't modify any package.

I addressed the other changes that you mentioned. Could you take another look at this PR?

joshuatf

Looks great, Fernando! Testing well; left a few comments, but only 1 I think we really need to handle before merging.

joshuatf · 2022-11-21T15:04:30Z

plugins/woocommerce-admin/client/products/sections/images-section.tsx

-												...files,
-											] );
-										}
+								<FormFileUpload


I'm good with this here for now, but just curious if we want all media upload components to have the same behavior with file uploading (cc @jarekmorawski).

If so, we could move this into the MediaUploader component. But we can save that for a follow-up if we need to do that.

Yes. We should use the same interactions across all media upload components. 👍

we could move this into the MediaUploader component. But we can save that for a follow-up if we need to do that.

Good idea. @joshuatf since this PR has been open for so long, I would add that change in a follow-up PR.

joshuatf · 2022-11-21T15:05:41Z

plugins/woocommerce-admin/client/products/sections/images-section.tsx

+												allowedMediaTypes={ [
+													ALLOWED_MEDIA_TYPES,
+												] }
+												onError={ () => null }


Not directly related to this PR but we should probably add some error handling at some point. I'm not sure if @mdperez86 already has an issue open for this or not as I know he implemented some error handling previously.

joshuatf · 2022-11-21T15:12:49Z

plugins/woocommerce-admin/client/products/sections/images-section.scss

+		.woocommerce-form-file-upload {
+			display: flex;
+			justify-content: center;
+			margin: -4em -2.5em -2.5em;


I'm a little hesitant on adding negative margins. This currently goes outside the parent container a bit.

Could we set .components-card__body to 0 padding when there are no images? We'll also need to set the margin on .woocommerce-sortable to 0 when there are no images.

joshuatf

Changes look good and still testing well! 🚢

Can you handle opening a follow-up issue to move the file upload component and styles that we discussed here?

octaedro · 2022-11-24T15:25:07Z

Hi @joshuatf,

I just rebased this PR and created the issue mentioned in this comment. Could you take another look at this PR?

AnnaMag

LGTM and tests well. 🚢

github-actions bot added focus: react admin package: @woocommerce/components issues related to @woocommerce/components plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Oct 27, 2022

octaedro changed the title ~~Add/35183 open file picker by clicking card~~ Open file picker by clicking card Nov 1, 2022

octaedro requested a review from a team November 1, 2022 19:32

octaedro marked this pull request as ready for review November 1, 2022 19:34

octaedro requested review from a team and removed request for a team November 1, 2022 19:35

octaedro commented Nov 2, 2022

View reviewed changes

joshuatf reviewed Nov 2, 2022

View reviewed changes

octaedro commented Nov 4, 2022

View reviewed changes

packages/js/components/src/media-uploader/README.md Outdated Show resolved Hide resolved

joshuatf reviewed Nov 4, 2022

View reviewed changes

octaedro force-pushed the add/35183_open_file_picker_by_clicking_card branch from 7839d0e to eddce5d Compare November 16, 2022 10:54

github-actions bot removed the package: @woocommerce/components issues related to @woocommerce/components label Nov 16, 2022

octaedro requested a review from joshuatf November 17, 2022 14:05

joshuatf reviewed Nov 21, 2022

View reviewed changes

octaedro self-assigned this Nov 22, 2022

octaedro force-pushed the add/35183_open_file_picker_by_clicking_card branch from a1e7681 to 981a553 Compare November 23, 2022 00:43

joshuatf previously approved these changes Nov 23, 2022

View reviewed changes

octaedro added 4 commits November 24, 2022 11:34

Add FormFileUpload component

c07c379

Add styles

4e2ebe6

Add changelog

dfe2133

Fix lint

43c7366

octaedro dismissed joshuatf’s stale review via 43c7366 November 24, 2022 14:37

octaedro force-pushed the add/35183_open_file_picker_by_clicking_card branch from ad9b4fa to 43c7366 Compare November 24, 2022 14:37

octaedro mentioned this pull request Nov 24, 2022

[Enhancement]: move file picker by clicking card into the MediaUploader component #35714

Closed

AnnaMag self-requested a review November 25, 2022 11:00

AnnaMag approved these changes Nov 25, 2022

View reviewed changes

octaedro merged commit 12121a4 into trunk Nov 25, 2022

octaedro deleted the add/35183_open_file_picker_by_clicking_card branch November 25, 2022 12:07

github-actions bot added this to the 7.3.0 milestone Nov 25, 2022

github-actions bot mentioned this pull request Jan 13, 2023

Test WooCommerce 7.3 UVLabs/location-picker-at-checkout-for-woocommerce#59

Closed

Open file picker by clicking card #35358

Open file picker by clicking card #35358

Uh oh!

Conversation

octaedro commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

github-actions bot commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

octaedro commented Nov 2, 2022

Uh oh!

jarekmorawski commented Nov 2, 2022

Uh oh!

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro commented Nov 16, 2022

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshuatf left a comment

Choose a reason for hiding this comment

Uh oh!

octaedro commented Nov 24, 2022

Uh oh!

AnnaMag left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

octaedro commented Oct 27, 2022 •

edited

Loading

github-actions bot commented Oct 27, 2022 •

edited

Loading