Update drop zone to Polaris v3.10.0 #299

andrewpye · 2019-03-26T13:36:51Z

Adds the React implementation's reset inputHTMLElement to allow duplicate files change.

andrewpye · 2019-03-26T13:38:05Z

vladucu · 2019-03-27T10:28:29Z

There are a bunch of other changes here from what I can see, or I'm wrong?

andrewpye · 2019-03-27T10:35:35Z

There are a bunch of other changes here from what I can see, or I'm wrong?

Wondering what those changes are in case I missed anything - just checked the diff again and the majority of the changes are import restructuring and updates to use the icon code changes which we don't have at this point (and can't have at this point, see Shopify/polaris#385 for details).

vladucu · 2019-03-27T10:44:51Z

Wondering what those changes are in case I missed anything

Shopify/polaris@v2.11.0...v3.10.0#diff-7ee058419e736ffe4c643519496b920fR426

andrewpye · 2019-03-27T10:49:42Z

lol... the diff is too big to show me what you're pointing out 😅 Were there any lines in particular that worried you? I'll accept a screen grab since github can't handle such large diffs 😉

vladucu · 2019-03-27T10:54:07Z

screen-shot

…is uploaded

andrewpye · 2019-03-27T16:01:08Z

@vladucu updated with the changes I missed earlier - thanks for spotting those! 🙌

There are a handful of new tests that were added for overlayText and errorOverlayText on different screen sizes - I'll add them in the morning 👍

tomnez

Do we want to update the file name attributes in polaris-drop-zone/file-upload too? We're still using IconDragDrop, AssetFileUpload, and AssetImageUpload.

andrewpye · 2019-03-28T10:43:27Z

@tomnez yeah I originally left those file name updates as part of the bigger icon update that we'll need to do in the future, but realised there wasn't really any reason not to update them now so that's done.

I've also added the tests I referred to yesterday, so this should be complete now 🙏

tomnez

🎉 Aw yiss

vladucu

Few small things and

@tomnez yeah I originally left those file name updates as part of the bigger icon update that we'll need to do in the future, but realised there wasn't really any reason not to update them now so that's done.

I don't see these changed...maybe a Github glitch?!?

addon/components/polaris-drop-zone.js

vladucu · 2019-03-28T18:01:58Z

addon/components/polaris-drop-zone.js

      dragging: false,
      error: rejectedFiles.length > 0,
    });
+    state.incrementProperty('numFiles', acceptedFiles.length);


nit: should we just move in the setProperties above instead of using incrementProperty?

vladucu · 2019-03-28T18:19:51Z

addon/templates/components/polaris-drop-zone/file-upload.hbs

@@ -13,11 +13,11 @@
 {{else if (eq size "large")}}
  {{#polaris-stack vertical=true spacing="tight"}}
    {{#if (eq type "file")}}
-      {{svg-jar assetFileUpload class=imageClasses}}
+      {{svg-jar fileUpload class=imageClasses}}


this might be my bad when initially implemented....but instead of an svg elements, we should be using img

don't think there was a specific reason for doing this

tomnez · 2019-03-28T18:24:28Z

@vladucu maybe you're looking at an older diff or something? I see the changes in addon/components/polaris-drop-zone/file-upload.js when I visit the diff tab.

vladucu · 2019-03-28T18:41:06Z

hmm, @tomnez I only see some renaming

tomnez · 2019-03-28T18:50:48Z

@vladucu yeah that was the change I requested. What else were you thinking was there?

Although it looks like the rename of iconDragDrop to DragDropMajorMonotone was missed 😕

Edit: Maybe when @andrewpye said "file name updates" he meant "variable name updates" since thats what I was asking about.

andrewpye · 2019-03-29T14:04:13Z

Although it looks like the rename of iconDragDrop to DragDropMajorMonotone was missed 😕

@tomnez this was deliberate - since Polaris switched to using @shopify/polaris-icons, all of their icon SVGs have changed name. We can't currently take those changes (discussion with Shopify is here) so we need to keep using the same old icon names for the time being, until we can start using the new icon formats.

vladucu · 2019-03-29T14:05:50Z

@tomnez this was deliberate - since Polaris switched to using @shopify/polaris-icons, all of their icon SVGs have changed name. We can't currently take those changes (discussion with Shopify is here) so we need to keep using the same old icon names for the time being, until we can start using the new icon formats.

ohh, so only the icon names changed here....but the actual icon didnt?

andrewpye · 2019-03-29T14:08:19Z

ohh, so only the icon names changed here....but the actual icon didnt?

TBH I haven't checked the new icon, but from what I've seen they've just changed the naming to be more explicit, and capitalised it to reflect the fact that the icons are now React components instead of SVG files. The way I see it, we need to make a fairly hefty overhaul to polaris-icon which we can't do yet so I'm thinking we leave icons as they are for now and revisit icon usages at the time of that update, thoughts?

vladucu · 2019-03-29T14:35:47Z

TBH I haven't checked the new icon, but from what I've seen they've just changed the naming to be more explicit, and capitalised it to reflect the fact that the icons are now React components instead of SVG files. The way I see it, we need to make a fairly hefty overhaul to polaris-icon which we can't do yet so I'm thinking we leave icons as they are for now and revisit icon usages at the time of that update, thoughts?

works for me...maybe add a TODO so we don't forget?

andrewpye · 2019-03-29T14:37:14Z

works for me...maybe add a TODO so we don't forget?

Should be pretty clear anyway, we'll need to update all polaris-icon usages so a TODO seems unnecessary 🤷‍♂️

vladucu

I think we gucci here ✨

andrewpye added 2 commits March 26, 2019 15:22

Allow duplicate files on drop zone

f7fa12c

Add drop zone test for duplicate files

a9ca229

andrewpye self-assigned this Mar 26, 2019

andrewpye requested review from vladucu and tomnez March 26, 2019 13:36

andrewpye added 3 commits March 27, 2019 16:57

Add selected file count tracking to drop zone

cfeaf54

Update drop zone utils to match React updates

63a91fd

Add callbacks test for drop zone without allow multiple after a file …

161ceb8

…is uploaded

tomnez reviewed Mar 27, 2019

View reviewed changes

andrewpye added 3 commits March 28, 2019 11:37

Update drop zone file upload asset names

c1dc52f

Add overlay text tests for drop zone

1a8324e

Add error overlay text tests for drop zone

21c7674

andrewpye requested a review from tomnez March 28, 2019 10:42

tomnez approved these changes Mar 28, 2019

View reviewed changes

vladucu requested changes Mar 28, 2019

View reviewed changes

Drop zone tweaks after review

b6997fc

Use img tags instead of svg-jar in drop zone file upload component

0d2bc84

vladucu approved these changes Mar 29, 2019

View reviewed changes

andrewpye merged commit a359913 into update/polaris-v3.10.0 Mar 29, 2019

delete-merged-branch bot deleted the update/drop-zone-v3.10.0 branch March 29, 2019 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update drop zone to Polaris v3.10.0 #299

Update drop zone to Polaris v3.10.0 #299

andrewpye commented Mar 26, 2019

andrewpye commented Mar 26, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

tomnez left a comment

andrewpye commented Mar 28, 2019

tomnez left a comment

vladucu left a comment

vladucu Mar 28, 2019

vladucu Mar 28, 2019

vladucu Mar 28, 2019

tomnez commented Mar 28, 2019

vladucu commented Mar 28, 2019

tomnez commented Mar 28, 2019 •

edited

andrewpye commented Mar 29, 2019

vladucu commented Mar 29, 2019

andrewpye commented Mar 29, 2019

vladucu commented Mar 29, 2019

andrewpye commented Mar 29, 2019

vladucu left a comment

Update drop zone to Polaris v3.10.0 #299

Update drop zone to Polaris v3.10.0 #299

Conversation

andrewpye commented Mar 26, 2019

andrewpye commented Mar 26, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

vladucu commented Mar 27, 2019

andrewpye commented Mar 27, 2019

tomnez left a comment

Choose a reason for hiding this comment

andrewpye commented Mar 28, 2019

tomnez left a comment

Choose a reason for hiding this comment

vladucu left a comment

Choose a reason for hiding this comment

vladucu Mar 28, 2019

Choose a reason for hiding this comment

vladucu Mar 28, 2019

Choose a reason for hiding this comment

vladucu Mar 28, 2019

Choose a reason for hiding this comment

tomnez commented Mar 28, 2019

vladucu commented Mar 28, 2019

tomnez commented Mar 28, 2019 • edited

andrewpye commented Mar 29, 2019

vladucu commented Mar 29, 2019

andrewpye commented Mar 29, 2019

vladucu commented Mar 29, 2019

andrewpye commented Mar 29, 2019

vladucu left a comment

Choose a reason for hiding this comment

tomnez commented Mar 28, 2019 •

edited