fix(file-uploader): change button from sm to default primary #2048

dakahn · 2019-03-11T21:58:29Z

partly Closes #2043

Removes the sm attribute on the uploader button so it meets the spec height of 48px.

Could the reviewers double check that the background color on uploaded files is properly field-01?

# Conflicts: # src/components/inline-loading/inline-loading.hbs

This reverts commit f87c085, reversing changes made to 130290e.

netlify · 2019-03-11T21:59:56Z

Deploy preview for the-carbon-components ready!

Built with commit 1bdf59f

https://deploy-preview-2048--the-carbon-components.netlify.com

netlify · 2019-03-11T22:01:30Z

Deploy preview for the-carbon-components ready!

Built with commit 919aebe

https://deploy-preview-2048--the-carbon-components.netlify.com

asudoh · 2019-03-11T22:40:10Z

src/components/file-uploader/file-uploader.hbs

@@ -11,7 +11,7 @@
  <p class="{{@root.prefix}}--label-description">only .jpg and .png files. 500kb max file size.</p>
  <div class="{{@root.prefix}}--file" data-file>
    <label for="your-file-importer-id-here"
-      class="{{@root.prefix}}--file-btn {{@root.prefix}}--btn {{@root.prefix}}--btn--primary {{@root.prefix}}--btn--sm"


Seems that this portion of the markup affects v9 as well - Can we rope it off?

dakahn · 2019-03-11T23:22:56Z

src/components/file-uploader/file-uploader.hbs

@@ -11,7 +11,7 @@
  <p class="{{@root.prefix}}--label-description">only .jpg and .png files. 500kb max file size.</p>
  <div class="{{@root.prefix}}--file" data-file>
    <label for="your-file-importer-id-here"
-      class="{{@root.prefix}}--file-btn {{@root.prefix}}--btn {{@root.prefix}}--btn--primary"
+      class="{{@root.prefix}}--file-btn {{@root.prefix}}--btn {{@root.prefix}}--btn--primary {{#if componentsX}}{{else}}{{@root.prefix}}--btn--sm{{/if}}"


@asudoh This feels a little sketchy -- let me know if there's a smarter way to do this. 👍

you may want to use unless here instead

Ah, exactly! Thanks 💪

src/components/file-uploader/file-uploader.hbs

Removes unneeded white space Co-Authored-By: dakahn <[email protected]>

asudoh

LGTM 👍 - Thanks @dakahn for all the updates!

laurenmrice

For some reason now I can see the background of the uploaded files by using the netlify link!

Changes:

The uploaded file box should be 40px/2.5rem not 32px.
There needs to be an icon-01 (#171717) 16px close icon in the uploaded file so a user can delete it. Needs to be 16px from the right hand side and have 16px padding from the left hand side.
We do not need the success checkmark icon in the uploaded file.
For the error state, the input text needs to be disabled-02 (#bebebe). Make sure the error message text is calling caption-01 (support-01, #da1e28). ~~The warning icon is pulling in the old icon, but this seems to be a global issue for components.~~ The icon is correct disregard this.
Capitalize O in 'Only'.

asudoh · 2019-03-13T03:18:58Z

@laurenmrice Thank you for your visual review (as always)! May I ask what you think the absolute no-go items for this PR are?

laurenmrice · 2019-03-13T14:54:43Z

@asudoh absolute no-go for this PR would be that we need to have a close button on each uploaded file.

dakahn · 2019-03-13T18:55:02Z

@laurenmrice It looks like the close X icon as well as the confirmation and error handling states and their icons are something we provide to consumers of this component -- but do not handle the functionality of on our end. So that first example is -- "hey, here's what we give you out of the box" and the example of the various states below it (success, failure, even X to remove uploaded file) is an example of some of the things they can do with our component.

@asudoh Looked at file-uploader.js with @joshblack and it looks like setState is a public API -- does this sound right?

laurenmrice

Ok thanks DA for explanation about close, success and loading icon examples! 👍

Changes for the examples:

So we can show a success example but we need to pull in the 16/checkmark and it should be color token interactive-01 (#0062ff) https://carbon-elements.netlify.com/icons/examples/preview/#16%2Fcheckmark
Also make sure the loading spinner is pulling in the one that is being used on the inline loading page.
The close icon, checkmark icon and loader need to be 16px away from the right.

asudoh · 2019-03-13T21:43:55Z

Looked at file-uploader.js with @joshblack and it looks like setState is a public API -- does this sound right?

Thank you for asking @dakahn - Yes, public API, and the close button shows up upon setState() call IIRC.

asudoh · 2019-03-13T22:03:56Z

Let's track the remainder in #2088 - Thanks @laurenmrice for working with on triaging each items!

carbon-bot · 2019-03-13T22:24:44Z

🎉 This PR is included in version 9.84.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

dakahn and others added 13 commits March 6, 2019 17:53

fix(inline-loading): adjust stroke width and svg width

f896874

chore(inline-loading): merge master

f5cee8c

fix(inline-loader): remove accidental changes

299ebb7

fix(footer): rolls back unintentional formatting changes

5977218

fix(footer): rolls back unintentional formatting changes

3274d03

fix(footer): remove whitespace

1b942dd

Merge branch 'master' into 1906-loading-inline-loading-tweaks

0c68ac5

Merge branch 'master' into 1906-loading-inline-loading-tweaks

60515e5

Merge branch 'master' of github.com:IBM/carbon-components

b4b45ae

Merge branch 'master' of github.com:dakahn/carbon-components

f87c085

# Conflicts: # src/components/inline-loading/inline-loading.hbs

Revert "Merge branch 'master' of github.com:dakahn/carbon-components"

18a0254

This reverts commit f87c085, reversing changes made to 130290e.

fix(file-uploader): change button from sm to default

1bdf59f

Merge branch 'master' into dak/file-uploader-audit-fixes

d956534

dakahn changed the title ~~Dak/file uploader audit fixes~~ fix(file-uploader): change button from sm to default primary Mar 11, 2019

dakahn mentioned this pull request Mar 11, 2019

[Audit] File uploader #2043

Closed

asudoh reviewed Mar 11, 2019

View reviewed changes

fix(file-uploader): rope off button type change

948733a

dakahn commented Mar 11, 2019

View reviewed changes

fix(file-uploader): use unless of if hbs

2c434b6

asudoh reviewed Mar 11, 2019

View reviewed changes

src/components/file-uploader/file-uploader.hbs Outdated Show resolved Hide resolved

Update src/components/file-uploader/file-uploader.hbs

3f601fc

Removes unneeded white space Co-Authored-By: dakahn <[email protected]>

asudoh approved these changes Mar 12, 2019

View reviewed changes

asudoh added status: one more review 👀 status: ready for review 👀 labels Mar 12, 2019

emyarod approved these changes Mar 12, 2019

View reviewed changes

emyarod removed status: one more review 👀 status: ready for review 👀 labels Mar 12, 2019

Merge branch 'master' into dak/file-uploader-audit-fixes

b88f700

laurenmrice self-requested a review March 12, 2019 14:47

jnm2377 added the status: visual review 🔍 label Mar 12, 2019

laurenmrice requested changes Mar 12, 2019

View reviewed changes

fix(file-uploader): change upload file height and remove check

875d906

asudoh added this to the v10.0-rc1 milestone Mar 13, 2019

laurenmrice requested changes Mar 13, 2019

View reviewed changes

asudoh mentioned this pull request Mar 13, 2019

File uploader: Remainder of audit fixes #2088

Closed

4 tasks

Merge branch 'master' into dak/file-uploader-audit-fixes

919aebe

asudoh merged commit 011e22b into carbon-design-system:master Mar 13, 2019

carbon-bot added the released label Mar 13, 2019

emyarod mentioned this pull request May 14, 2019

[FileUploader/FileUploaderButton, react, bug?]: FileUploader & FileUploaderButton component in react adds bx--btn--sm CSS style to the button which is not present in VanillaJS implementation #2715

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(file-uploader): change button from sm to default primary #2048

fix(file-uploader): change button from sm to default primary #2048

dakahn commented Mar 11, 2019 •

edited

Loading

netlify bot commented Mar 11, 2019

netlify bot commented Mar 11, 2019 •

edited

Loading

asudoh Mar 11, 2019

dakahn Mar 11, 2019 •

edited

Loading

emyarod Mar 11, 2019

dakahn Mar 11, 2019

asudoh left a comment

laurenmrice left a comment •

edited by dakahn

Loading

asudoh commented Mar 13, 2019

laurenmrice commented Mar 13, 2019

dakahn commented Mar 13, 2019 •

edited

Loading

laurenmrice left a comment •

edited

Loading

asudoh commented Mar 13, 2019 •

edited

Loading

asudoh commented Mar 13, 2019

carbon-bot commented Mar 13, 2019

fix(file-uploader): change button from sm to default primary #2048

fix(file-uploader): change button from sm to default primary #2048

Conversation

dakahn commented Mar 11, 2019 • edited Loading

netlify bot commented Mar 11, 2019

netlify bot commented Mar 11, 2019 • edited Loading

asudoh Mar 11, 2019

Choose a reason for hiding this comment

dakahn Mar 11, 2019 • edited Loading

Choose a reason for hiding this comment

emyarod Mar 11, 2019

Choose a reason for hiding this comment

dakahn Mar 11, 2019

Choose a reason for hiding this comment

asudoh left a comment

Choose a reason for hiding this comment

laurenmrice left a comment • edited by dakahn Loading

Choose a reason for hiding this comment

asudoh commented Mar 13, 2019

laurenmrice commented Mar 13, 2019

dakahn commented Mar 13, 2019 • edited Loading

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

asudoh commented Mar 13, 2019 • edited Loading

asudoh commented Mar 13, 2019

carbon-bot commented Mar 13, 2019

dakahn commented Mar 11, 2019 •

edited

Loading

netlify bot commented Mar 11, 2019 •

edited

Loading

dakahn Mar 11, 2019 •

edited

Loading

laurenmrice left a comment •

edited by dakahn

Loading

dakahn commented Mar 13, 2019 •

edited

Loading

laurenmrice left a comment •

edited

Loading

asudoh commented Mar 13, 2019 •

edited

Loading