-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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: Improve ARIA labelling fold controls #5205
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #5205 +/- ##
==========================================
+ Coverage 87.19% 87.20% +0.01%
==========================================
Files 565 565
Lines 45121 45161 +40
Branches 6910 6914 +4
==========================================
+ Hits 39342 39382 +40
Misses 5779 5779
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
demo/i18n.html
Outdated
"Unfold rows $0 to $1": "", | ||
"Code folding, rows $0 through $1": "", | ||
"Code folding, row $0": "", | ||
"Unfold code": "", | ||
"Fold at row $0": "", |
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.
if someone was using the old translation keys, what is going to happen? can we at least show a warning? can we support both by deprecating the old one?
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.
added a warning when a Ace tries to replace a string using the nls
function but can't find a translation in the provided messages.
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.
Does this mean this is a breaking change? If so, can we avoid it or is it intentional?
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 nice!
Description of changes: For a11y compliance, buttons that show/hide content should have an
aria-expanded
state. This adds that state and changes the aria label accordingly.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.