-
Notifications
You must be signed in to change notification settings - Fork 155
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: add loading a11y to s3 selectors for screen readers #2475
feat: add loading a11y to s3 selectors for screen readers #2475
Conversation
4aa8453
to
47eb27c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2475 +/- ##
==========================================
- Coverage 95.80% 95.80% -0.01%
==========================================
Files 728 728
Lines 19874 19886 +12
Branches 6700 6745 +45
==========================================
+ Hits 19040 19051 +11
- Misses 779 827 +48
+ Partials 55 8 -47 ☔ View full report in Codecov by Sentry. |
8bd178a
to
c54cc6c
Compare
c54cc6c
to
3b3b8c7
Compare
3b3b8c7
to
8dcb8c8
Compare
9d5cc02
to
6dc8f95
Compare
6dc8f95
to
dcb11fe
Compare
dcb11fe
to
a5680a1
Compare
return ( | ||
<InternalSpaceBetween size="s" direction="horizontal" alignItems="center"> | ||
{getLastUpdated()} | ||
<InternalButton iconName="refresh" ariaLabel={i18nStrings.labelRefresh} onClick={reloadData} /> |
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.
Just a random comment - it's crazy to me that we don't disable the reload button while a fetch is taking place. But at this point, I'm a little worried that switching it to a loading state is a breaking change, so... 😔
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.
It certainly looks weird not receiving a feedback when clicking on the button. I think here the other parts of the "loading & refreshing" guideline also comes into play.
Cases for when it's loading, it's taking longer that usual or it's failing for some reason.
I think it makes sense to bucket them together, in another PR.
a5680a1
to
d741935
Compare
d741935
to
f82ac50
Compare
Description
Adds
Last Updated
to the S3 resource selector modal.The addition of this text should help the screen readers to inform the users once the fetch process is finished.
Related links, issue #, if available: AWSUI-41521
How has this been tested?
Manually tested on Chrome, Firefox & Safari.
Automated testing in progress.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.