-
Notifications
You must be signed in to change notification settings - Fork 38
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
[ENG-3009][ENG-3087][ENG-2977] Add aria-labels to sharing icons #456
[ENG-3009][ENG-3087][ENG-2977] Add aria-labels to sharing icons #456
Conversation
{{input | ||
(html-attributes aria-label='Search') |
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.
This has to be a bare string rather than a translated one due to limitations on ember-component-attributes
. Though I suspect this will be merged into ember-osf-web before we get to translating preprints to a new language.
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.
A bit curious why this didn't work
(html-attributes aria-label='Search') | |
(html-attributes aria-label=(t 'eosf.components.search')) |
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.
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 great! some minor suggestions
{{input | ||
(html-attributes aria-label='Search') |
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.
A bit curious why this didn't work
(html-attributes aria-label='Search') | |
(html-attributes aria-label=(t 'eosf.components.search')) |
Co-authored-by: Fabrice Mizero <fabrice@cos.io>
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.
⭐
## Purpose Fix some more accessibility problems on the preprint detail page ## Summary of Changes/Side Effects <!-- What did you change? Include before/after screenshots or a video/gif if applicable. Do these changes have any unforseen effects (good or bad)?--> ## Testing Notes 1. Update version of ember-osf to fix sharing icons not having aria-labels 2. Remove redundant `id='view-page'` 3. Add expand/collapse aria label to mfr expand/collapse control ## Notes for Reviewer This will need to be yarn updated and ci run after CenterForOpenScience/ember-osf#456 is deployed. Also, this does not fix the MFR iframe issue, which looks to be an MFR problem.
## Purpose Fixes the accessibility problems related to contrast on the Preprints submit/edit page. Some of these changes (`(required)` red in the license picker items) are in CenterForOpenScience/ember-osf#456 but this PR isn't pinned to that, as another preprints PR is already pinned. ## Summary of Changes/Side Effects 1. Darker black for the preprint header preview `<i>` tag 2. Darker red for `(required)` and error messages 3. Darker placeholder text ## Testing Notes Should just need to make sure that above items are passing a11y guidelines. ## Ticket https://openscience.atlassian.net/browse/ENG-2977
Purpose
Fixes an accessibility problem on the ember preprints discover page (And also another on the preprints create/edit pages)
Summary of Changes/Side Effects
(required)
labels for https://openscience.atlassian.net/browse/ENG-2977Testing Notes
This will be tested alongside the ember-osf-preprints PR that this will be linked to.
Ticket
https://openscience.atlassian.net/browse/ENG-3009
https://openscience.atlassian.net/browse/ENG-3087
https://openscience.atlassian.net/browse/ENG-2977
Reviewer Checklist
CHANGELOG.md