-
Notifications
You must be signed in to change notification settings - Fork 55
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
Accessibility improvements #126
Conversation
2ddd555
to
0257d9a
Compare
I'm seeing a really weird issue on this. When I hit space in the editor, it's taking me to the beginning of the line. |
codemirror/codemirror5#6651 fixes an issue when CodeMirror is in a shadow root and using contenteditable mode. It's not yet released, so temporarily depend on the git commit. Note we generate and distribute our own CodeMirror build, so this is only a devDependency so it won't cause any versioning issues for consumers.
This turned out to be a CodeMirror + Shadow DOM compatibility bug. I've fixed it upstream in codemirror/codemirror5#6651, which is already merged. Since we generate our own CodeMirror build, for now I've just updated our |
Also got the |
} | ||
|
||
#keyboardHelp { | ||
background: #00000099; |
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.
can we get a little more line spacing and a little less left/right padding?
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.
.CodeMirror { | ||
height: 100% !important; | ||
font-family: inherit !important; | ||
border-radius: inherit; | ||
} | ||
|
||
#keyboardHelpScrim { |
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.
wdyt about some background shading here so that the help message is more obviously associated with the source editor?
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.
Done in #127
${this._showKeyboardHelp | ||
? html`<div id="keyboardHelpScrim"> | ||
<p id="keyboardHelp"> | ||
Press Enter to start editing<br />Press Escape to exit editor |
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.
Consider styling on "Enter" and "Escape" to make them stand out a bit? Maybe bold or monospace?
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.
Done in #127
…irtualizer-directive Apply fix from pull request google#120 to directive implementation
Live at https://polymerlabs.github.io/playground-elements/
Don't immediately enter edit mode when focusing the editor with Tab, because otherwise we're a Tab trap, because Tabs insert literal tabs. Instead, show a prompt to "Press Enter to start editing". Clicking into editable region with mouse works like before. Pressing Escape re-focuses the editor, allowing Tab to be used to focus next again.
Switch to CodeMirror's
contenteditable
input style. This makes it behave like a normal content editable region, so it's much better for screen readers (see https://codemirror.net/doc/manual.html#option_inputStyle -- it's the new default in CodeMirror 6 for this reason).Mark line numbers with
aria-hidden
so that they don't get read aloud by screen readers (see VoiceOver speaks line numbers codemirror/codemirror5#6578).