Skip to content
This repository has been archived by the owner on Nov 11, 2021. It is now read-only.

speech synth a11y edits #340

Merged
merged 18 commits into from
Jan 17, 2017
Merged

Conversation

ststimac
Copy link
Contributor

@ststimac ststimac commented Jan 5, 2017

This PR takes care of the following speech synthesis demo a11y issues:
#183
#218
#232
#285

I also cleaned up the CSS & updated units from px to rem to be consistent w/other demos.

@dstorey @melanierichards


input[type="text"]:focus,
select:focus {
border: 2px solid #ffa940;
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that the range inputs don't have a focus state in these updates. It doesn't look like you can put a border on them, cross-browser, so I'd propose making the focus state for the selects and range inputs:

outline: 1px dashed #fff;
outline-offset: 1px;

Would need to be tested in all the high contrast themes on Windows, not sure if the outline will disappear on the light HC themes.

@@ -19,40 +19,40 @@
</div>
<div id="container">
<div id="text-container">
<input type="text" name="textToSpeech" id="js-text-to-speech">
<input type="text" name="textToSpeech" id="text-to-speech" placeholder="Enter some text to turn into speech" aria-label="input text">
Copy link
Contributor

Choose a reason for hiding this comment

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

@dstorey I feel like this could have a more descriptive aria label but not sure what suggestion to make that wouldn't be redundant with the placeholder. Do you have any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@melanierichards @dstorey what if I change the placeholder text to an example phrase, and then use the current placeholder text as the aria-label.

From w3:
"Check that the value of the aria-label attribute properly describes the purpose of an element where user input is required"

placeholder
"Represents a short hint (a word or short phrase) intended to aid the user with data entry."

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that sounds good to me!

@ststimac
Copy link
Contributor Author

@melanierichards @dstorey focus styles on the range have been added + modified for the select. Aria-label and placeholder updated. Still need to test in HC.

@ststimac
Copy link
Contributor Author

Made adjustments for high contrast.

Also messed something up when I tried to update from remote. Had to fix a bunch of conflicts that don't relate to this demo.

@melanierichards
Copy link
Contributor

I feel like we've run into one of those fussy situations whereby making something more accessible for some we risk making it a little more confusing for others, in that it could be likely for someone to mistake the placeholder text for a filled-in value.

My suggestions would be:

  • Let's just add a text <label> above the speech balloon in lieu of the aria label on the input. "Enter some text to turn into speech:" I think erring on the side of clarity would be good (if you want to pump up the hierarchy of the speech balloon, you could make the padding puffier, but I think the hierarchy will be fine).
    * Perhaps add "Example:" to the beginning of the placeholder?
  • Perhaps make the placeholder italics?

@ststimac
Copy link
Contributor Author

OK, added the label and added "example" to the beginning of the placeholder.

@melanierichards
Copy link
Contributor

@ststimac cool! There's a conflict in the microphone.html file, and possibly others, but otherwise this will be good to go.

@ststimac
Copy link
Contributor Author

OK, should be good to go.

Copy link
Member

@dstorey dstorey left a comment

Choose a reason for hiding this comment

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

Looks good. Just minor change of not needed type="text/css"

@@ -6,8 +6,8 @@
<meta name="keywords" content="performance, settimeout, setimmediate, yielding, javascript, es5, ecmascript"/>
<meta name="og:title" content="JavaScript yielding"/>
<title>Efficient Script Yielding in JavaScript</title>
<link rel="stylesheet" href="https://edgeportal.blob.core.windows.net/media/demotemplate.css"/>
<link rel="stylesheet" href="styles/demo.css"/>
<link rel="stylesheet" type="text/css" href="//edgeportal.blob.core.windows.net/media/demotemplate.css"/>
Copy link
Member

Choose a reason for hiding this comment

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

These are not needed as CSS is the only kind of stylesheet.

@ststimac
Copy link
Contributor Author

@dstorey removed those type="text/css"

@dstorey dstorey merged commit 2c69b93 into MicrosoftEdge:master Jan 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants