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

feat(FileUploader): support button variants and container sizes #5614

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Mar 12, 2020

Closes #5436

This PR adds file uploader button variants as well as container size support (for upload button and uploaded items).

Changelog

New

  • pass size prop from FileUploaderButton to FileUploader
  • button variant support in FileUploader

Changed

  • default and drag and drop uploader documentation
  • codesandbox examples

Testing / Reviewing

Ensure the upload button/uploaded item elements are the correct size and ensure that different button variants are appearing correctly in the default uploader

@emyarod emyarod requested a review from a team as a code owner March 12, 2020 18:45
@ghost ghost requested review from asudoh and dakahn March 12, 2020 18:45
@netlify
Copy link

netlify bot commented Mar 12, 2020

Deploy preview for carbon-components-react ready!

Built with commit 3f1ccd1

https://deploy-preview-5614--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Mar 12, 2020

Deploy preview for carbon-elements ready!

Built with commit 3f1ccd1

https://deploy-preview-5614--carbon-elements.netlify.com

@laurenmrice
Copy link
Member

is it possible to only include the primary and tertiary button options in the dropdown for this component? I would rather not let people have the option to change it to the others. for consistency

@emyarod
Copy link
Member Author

emyarod commented Mar 12, 2020

sure, restricted button types to primary and tertiary

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

For the drag and drop uploader, can we make it so the file extends always to the same width as the drag drop box? Right now it seems the file is dependent on the length of text inside.
Screen Shot 2020-03-12 at 2 44 29 PM

@emyarod emyarod force-pushed the 5436-file-uploader-element-sizes branch from 6eaebf7 to 3bc7fc8 Compare March 13, 2020 14:10
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

looks good to me ! thanks Andrew 🙌🏻

* Specify the size of the button, from a list of available sizes.
* For `default` buttons, this prop can remain unspecified.
*/
size: PropTypes.oneOf(['default', 'field', 'small']),
Copy link
Contributor

Choose a reason for hiding this comment

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

@tw15egan @laurenmrice Any thoughts on the list of size variant names? Just wanted to see if we have a chance to align this with some other components. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Text input, Number input, Dropdown and Combobox all currently have this format (Extra large size (XL), Default size, Small size (sm)). While Button has this format (Default, Field, Small). Both are inconsistent to describe the same thing.

For file uploader, the drag and drop gets a knob for filename size, while the regular file uploader gets one knob for both button and a filename size because they need to scale together. I think for now it is probably fine how we have it, we are planning on reevaluating the naming convention of these components as apart of v11 to keep them all consistent, and that would fix this problem.

Copy link
Member

@laurenmrice laurenmrice Mar 16, 2020

Choose a reason for hiding this comment

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

We would eventually want it to be something like large, medium/(default), and small

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied the field and small naming over from Button since the file uploader is using a button but I can change it to lg, md/default, and sm. would we want that instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I know, I am thinking if we change this one to a new naming convention right now it may confuse people even more because then it is different from the current button component.

Copy link
Member Author

Choose a reason for hiding this comment

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

got it, I will leave it for now

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod!

@asudoh
Copy link
Contributor

asudoh commented Mar 17, 2020

@emyarod thanks this is ready to merge - Thanks @emyarod!

@asudoh asudoh merged commit 5c2bf2a into carbon-design-system:master Mar 17, 2020
@emyarod emyarod deleted the 5436-file-uploader-element-sizes branch March 17, 2020 14:38
renmaddox added a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
renmaddox added a commit to renmaddox/carbon that referenced this pull request Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[file uploader] option to change button and uploaded file sizes
4 participants