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

fix(FileUploader): resolve component audit issues #7474

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Dec 16, 2020

Closes #7448

This PR updates the file uploader examples to resolve component audit bugs. A few questions about the audit points:

Alert icon needs a black inner fill for dark themes.

for the black fill within alert icons on dark themes, we run into the same issue with notifications from before (#3216) except we're not able to force file uploaders to be on dark backgrounds. alternatively we can just apply the black fill across the board on all themes if that's fine

The container needs to be disabled-02.

which container is this referring to?

Testing / Reviewing

Confirm the file uploaders match the updated spec

@emyarod emyarod requested a review from a team as a code owner December 16, 2020 20:27
@emyarod emyarod changed the title 7448 file uploader audit fixes fix(FileUploader): resolve component audit issues Dec 16, 2020
@netlify
Copy link

netlify bot commented Dec 16, 2020

✔️ Deploy preview for carbon-elements ready!

🔨 Explore the source changes: 1a954fe

🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-elements/deploys/5ff38266b8b0fb000721b372

😎 Browse the preview: https://deploy-preview-7474--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Dec 16, 2020

✔️ Deploy preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 1a954fe

🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-components-react/deploys/5ff38266586db90007ef763f

😎 Browse the preview: https://deploy-preview-7474--carbon-components-react.netlify.app

Copy link
Member

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

@emyarod emyarod force-pushed the 7448-file-uploader-audit-fixes branch from e1c023e to 21ced0c Compare December 17, 2020 16:00
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.

Default file uploader

  • Header needs to say “Upload files”
  • Button text needs to say “Add file"

Uploaded File
Invalid state

  • There is still a shift down when the invalid state is applied. The background of the field should increase in height to accommodate the error message below it. But the text and icons in the field should stay in the same place and not be jumping down.
    invalid state

  • There needs to be a 8px space between the alert icon and the close icon.
    Artboard

File Uploader story

  • The length of the file changes in width depending on the length of the text inside of it. Can we fix this so the file always has a width of 288px like it does in the other examples we show?

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.

The invalid state looks perfect for the default size file! It is still jumping down for the field and small sizes.

@emyarod emyarod force-pushed the 7448-file-uploader-audit-fixes branch from fb9f7ab to b374e52 Compare December 17, 2020 18:40
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 great! 👏

@kodiakhq kodiakhq bot merged commit 2ebb0fe into carbon-design-system:master Jan 4, 2021
@emyarod emyarod deleted the 7448-file-uploader-audit-fixes branch January 7, 2021 20:03
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] component audit bugs
3 participants