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(file-uploader): markup and experimental changes #1450

Merged
merged 9 commits into from
Nov 26, 2018
Merged

feat(file-uploader): markup and experimental changes #1450

merged 9 commits into from
Nov 26, 2018

Conversation

aledavila
Copy link
Contributor

@aledavila aledavila commented Nov 16, 2018

Closes #1296

File Upload

Changelog

Changed

  • experimental colors
  • close button svg

Testing / Reviewing

http://file-uploader-x-balanced-hyena.mybluemix.net/?nav=file-uploader

** Additional functionality won't be added until after release

Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

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

✋ Hello! Happy Friday. Two things I picked up:

Keyboard nav is broken. Testing in Firefox and Chrome when the user tabs to the button they can't actually use space or enter to activate it -- they have to tab one level deeper into the button to actually throw the uploader modal.

There's a DAP violation about incorrectly assigning a label element the aria role="button". Here's the link to the documentation for the violation

These are out of the scope of this PR and I'm addressing them in this PR.

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 basically, just a comment - Thanks @aledavila!

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.

Thanks @aledavila for your quick update! Good to go once design review finishes.

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 only thing I noticed is that the close (X) icon is not pulling in correctly. Make sure we are using the 16/close icon here https://ibm.github.io/carbon-elements/icons/examples/esm/#16%2Fclose
. Everything else looks good!

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!  👍

@aledavila aledavila merged commit d2ec90a into carbon-design-system:master Nov 26, 2018
@carbon-bot
Copy link
Contributor

🎉 This PR is included in version 9.55.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Experimental Component - File Uploader - Development
7 participants