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

[Typography] Add inherit and screen reader only #12837

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Sep 11, 2018

  • variant="srOnly" use case:
import Icon from '@material-ui/core/Icon';
import Typography from '@material-ui/core/Typography';

// ...

<Icon>add_circle</Icon>
<Typography variant="srOnly">Create a user</Typography>
  • variant="inherit" use case:
            <MenuItem>
              <Typography variant="inherit" noWrap>
                Label
              </Typography>
            </MenuItem>

@oliviertassinari oliviertassinari changed the title [TypeScript] Add inherit and screen reader only [Typography] Add inherit and screen reader only Sep 11, 2018
@oliviertassinari oliviertassinari force-pushed the typography-sronly-inherit-variants branch from 857ba89 to 5c02039 Compare September 11, 2018 08:10
@oliviertassinari oliviertassinari added new feature New feature or request component: Typography The React component. labels Sep 11, 2018
@mbrookes
Copy link
Member

<Icon aria-label="Create a user">add_circle</Icon>?

@oliviertassinari
Copy link
Member Author

@mbrookes I haven't tried with a screen reader, it might not work. I can only find reference to stuff like http://web-accessibility.carnegiemuseums.org/code/svg/.

@mbrookes
Copy link
Member

I should have checked the literature rather than just assuming, however:

"SVG user agents MUST provide an accessible object in the accessibility tree for rendered SVG elements that meet any of the following criteria [...]:

  • It has at least one direct child ‘title’ element or ‘desc' element that is not empty after trimming whitespace. User agents MAY include elements with these child elements without checking for valid text content.
  • It has a non-empty (after trimming whitespace) ‘aria-label’ attribute or ‘aria-roledescription’ attribute.
    [...]

https://www.w3.org/TR/svg-aam-1.0/#include_elements

(which doesn't mean srOnly isn't potentially useful elsewhere)

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 11, 2018

It's for Font icons right, not SVG?


What do you prefer between srOnly and screenReaderOnly?

@mbrookes
Copy link
Member

🤦‍♂️
In that case, no, aria-label won't work as we apply aria-hidden to the icon span. However wouldn't you be using an IconButton in this case?

screenReaderOnly while more verbose is much clearer as to what it does.

packages/material-ui/src/Backdrop/Backdrop.js Show resolved Hide resolved
@@ -6,7 +6,7 @@ export function isBody(node) {
return node && node.tagName.toLowerCase() === 'body';
}

// Do we have a scroll bar?
// Do we have a veritcal scroll bar?
Copy link
Member

Choose a reason for hiding this comment

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

Typo in vertical

@@ -32,6 +32,13 @@ export const styles = theme => ({
caption: theme.typography.caption,
/* Styles applied to the root element if `variant="button"`. */
button: theme.typography.button,
/* Styles applied to the root element if `variant="srOnly"`. Only targets the screen readers. */
Copy link
Member

Choose a reason for hiding this comment

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

"Only accessible to screen readers" would be better

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 11, 2018

screenReaderOnly while more verbose is much clearer as to what it does.

I agree, however. The CSS API can help with the purpose of this class name. Bootstrap & Font Awesome are using the .sr-only wording. It's something people are already used to 🤔.

@oliviertassinari oliviertassinari merged commit 046fc98 into mui:master Sep 12, 2018
@oliviertassinari oliviertassinari deleted the typography-sronly-inherit-variants branch September 12, 2018 06:28
marcelpanse pushed a commit to marcelpanse/material-ui that referenced this pull request Oct 2, 2018
* [Typography] Add inherit and screen reader only

* review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Typography The React component. new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants