Skip to content
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

Minor changes to make CodeMirror accessible (label and color-contrast) #6920

Merged
merged 4 commits into from May 28, 2022

Conversation

kudelek
Copy link
Contributor

@kudelek kudelek commented May 24, 2022

Minor change made for the project to be a bit more accessible ('label' violation)

el("span", {className: "CodeMirror-search-label"}, cm.phrase("Search:")), " ",
el("input", {type: "text", "style": "width: 10em", className: "CodeMirror-search-field"}), " ",
label, " ",
el("input", {type: "text", "style": "width: 10em", className: "CodeMirror-search-field", id: "CodeMirror-search-field"}), " ",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a fixed ID here will break if there are multiple editors in the page. Maybe just wrap the input in the label instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -131,5 +131,6 @@ export function hiddenTextarea() {
// If border: 0; -- iOS fails to open keyboard (issue #1287)
if (ios) te.style.border = "1px solid black"
disableBrowserMagic(te)
te.setAttribute("aria-hidden","true")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding the input element from the screen reader seems a very bad idea. What is the purpose of this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I probably misunderstood what is this textarea for. Maybe it would be good to just add aria-label for this textarea?
If so, what should be in the aria-label?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates the main focused element when inputMode is "textarea". Its label can be set with this option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I reverted this change and set the label locally. That's all for this pull request

@kudelek kudelek changed the title Minor changes to make CodeMirror accessible (label) Minor changes to make CodeMirror accessible (label and color-contrast) May 26, 2022
@kudelek kudelek requested a review from marijnh May 27, 2022 08:42
@marijnh marijnh merged commit e024ab4 into codemirror:master May 28, 2022
marijnh added a commit that referenced this pull request May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants