Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Linting support for hds-code-editor modifier and component #2715
base: main
Are you sure you want to change the base?
Linting support for hds-code-editor modifier and component #2715
Changes from 33 commits
b7395f6
1eb850b
6ea4d16
3397427
10f22f0
d2f0293
05ac3c9
d6d1d9e
fb36cb7
06ea6ad
b9b0d44
2855192
f34eb02
dadccec
876ca47
c9f9cee
24692fa
900be46
3115abd
9d49d90
399b6a7
f4d245a
f684452
f601cf0
f13e484
ac8596f
7755ad1
5cfcab9
9996115
b3fff15
3897372
c7194b6
62711d0
e13e01d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Issue: there's not a way for users to know what the keyboard shortcut is to open the drawer. At a minimum, we should add some screen reader only text between the header and the editor what the shortcut is and what it does.
Maybe @LilithJames-HDS could weigh in, but it would be nice if the instructions were available to sighted users too.
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.
Thinking about it more, this is especially important because the drawer is more accessible than the tooltips - so we want to make it super easy to know how to open it.
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.
Issue: the tooltips are currently not accessible and I'm not seeing an easy way to improve it in their docs...
The issues are:
One issue that we can fix in this PR is to add alt text for each icon. Currently there's no text, its just the icon. The text could be like "Syntax error on line {line number}" or something like that.
For the other issues, it may be best to just mark the component as not compliant if you use linting and file an issue with CodeMirror.
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.
Ah one more thing: the icon is currently showing up in the a11y tree as an image. This can be fixed by adding
aria-hidden="true"
to the element with the classcm-lint-marker-error
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.
[Issue] There are 2 a11y issues with the gutter, and I see them in the code mirror example as well. They could be solved by having focus be trapped inside the gutter and controlling where it gets set when closed, but idk if that is possible to customize.
Issue 1: Focus hidden from view
Shift-tab
moves focus from gutter back to previously focused lineIssue 2: Focus lost when closing gutter
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.
When you say gutter, do you mean the linting gutter (the bar to the side of the line numbers) or the drawer that you can open up with the list of errors?
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.
Ah sorry I meant the drawer that opens up