Skip to content
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.

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

Add FinalFormFileSelect #1461

Merged
merged 33 commits into from
May 14, 2024
Merged

Add FinalFormFileSelect #1461

merged 33 commits into from
May 14, 2024

Conversation

NataliaVizintini
Copy link
Contributor

@NataliaVizintini NataliaVizintini commented Nov 29, 2023

COM-81 - File Upload Form Field

FileUpload Field with Dropzone

Screenshot 2024-04-17 at 13 07 33

@johnnyomair
Copy link
Collaborator

@NataliaVizintini is this really draft? Or do you want a review?

@NataliaVizintini
Copy link
Contributor Author

NataliaVizintini commented Dec 12, 2023

@NataliaVizintini is this really draft? Or do you want a review?

@johnnyomair as I mentioned in our conversation, I need your feedback to continue the development, because I don't think the component is ready yet. Therefore it's a draft and I want a review, please.

@NataliaVizintini NataliaVizintini changed the title Draft: Add File Upload Form Field [POC] Add File Upload Form Field Dec 12, 2023
@vivid-planet vivid-planet deleted a comment from netlify bot Dec 13, 2023
@vivid-planet vivid-planet deleted a comment from netlify bot Dec 13, 2023
@johnnyomair johnnyomair changed the title [POC] Add File Upload Form Field [RFC] Add File Upload Form Field Dec 14, 2023
codemods/1.0.0/component-renames.ts Outdated Show resolved Hide resolved
packages/admin/admin-icons/icons/select.svg Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormFileUpload.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@jamesricky jamesricky left a comment

Choose a reason for hiding this comment

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

There seem to be a few styling differences between the design and the implementation:

Design Implementation
file upload design file upload implementation
file upload error design file upload error implementation
  • The height and border-color of the droppable area
  • The button color
  • The height of the items in the uploaded files list
  • The space between the uploaded files list and the "Maximum file size" text
  • The icon used in the error state should be a "!" but is an "i"

Also, the min-width's prevent the field from shrinking beyond a certain width:

This seems unnecessary to me.
Is there a specific reason for this, or could we remove that?

file.upload.resizing.mov

packages/admin/admin/src/form/FinalFormChooseFile.tsx Outdated Show resolved Hide resolved
packages/admin/admin/src/form/FinalFormChooseFile.tsx Outdated Show resolved Hide resolved
@jamesricky
Copy link
Contributor

FinalFormFileUpload makes the most sense to me, but that's exactly where we started.

Ah, just saw the comment from @johnnyomair, that makes sense.

Then I think FinalFormFileInput makes the most sense, would also be fine with FinalFormFileSelect though.

@NataliaVizintini
Copy link
Contributor Author

NataliaVizintini commented Apr 17, 2024

@jamesricky

  • The button color - I thought it should be primary? It would look very strange if all the buttons in a form are primary and this one is black? And the option with black is missing in button colours…
  • The height of the items in the uploaded files list - it’s the Chip height, when we’ll get Chip with new design, the height should be just as in design. But for now, it looks bad without the padding. (see screenshot)
Screenshot 2024-04-17 at 10 25 49

Min-width was discussed with Mitch, and we agreed it should be 250px for FileItem and DroppableArea.

@jamesricky
Copy link
Contributor

The button color - I thought it should be primary? It would look very strange if all the buttons in a form are primary and this one is black? And the option with black is missing in button colours…

Please check with the UX-Team if you're unsure if the design is correct.

@thomasdax98 The black button seems to be the new secondary variant in the design, in the current implementation (in main) it is green. Does the black button exist somewhere yet? Is there an open PR/Task for that?

The height of the items in the uploaded files list - it’s the Chip height, when we’ll get Chip with new design, the height should be just as in design. But for now, it looks bad without the padding. (see screenshot)

Good point, we'll leave it as is then for now 👍🏼

The space between the uploaded files list and the "Maximum file size" text - it’s 10px gap between all elements of the column, just as in design 🤔

There is also a margin in addition to the gap, so the total space is more than 10px.

Min-width was discussed with Mitch, and we agreed it should be 250px for FileItem and DroppableArea.

👍🏼

@NataliaVizintini NataliaVizintini changed the title Add File Upload Form Field ( FinalFormChooseFileField ) Add File Upload Form Field ( FinalFormFileSelect ) Apr 17, 2024
@johnnyomair johnnyomair removed the request for review from nsams April 18, 2024 14:42
NataliaVizintini and others added 2 commits April 23, 2024 13:11
Co-authored-by: Johannes Obermair <48853629+johnnyomair@users.noreply.github.com>
@johnnyomair
Copy link
Collaborator

@thomasdax98 @jamesricky please review once again, thanks!
@NataliaVizintini please merge main to fix conflicts.

@NataliaVizintini NataliaVizintini force-pushed the fileUpload branch 2 times, most recently from 587ab81 to 5086faf Compare April 25, 2024 14:42
@johnnyomair johnnyomair changed the title Add File Upload Form Field ( FinalFormFileSelect ) Add FinalFormFileSelect May 14, 2024
@johnnyomair johnnyomair merged commit 52130af into main May 14, 2024
11 checks passed
@johnnyomair johnnyomair deleted the fileUpload branch May 14, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants