-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat(file-uploader): markup and experimental changes #1450
Conversation
There was a problem hiding this 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.
There was a problem hiding this 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!
There was a problem hiding this 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.
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
🎉 This PR is included in version 9.55.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #1296
File Upload
Changelog
Changed
Testing / Reviewing
http://file-uploader-x-balanced-hyena.mybluemix.net/?nav=file-uploader
** Additional functionality won't be added until after release