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

Cross gridicon: omit title element? #205

Closed
bluefuton opened this issue May 19, 2017 · 11 comments
Closed

Cross gridicon: omit title element? #205

bluefuton opened this issue May 19, 2017 · 11 comments

Comments

@bluefuton
Copy link
Contributor

I've noticed that when we render the cross gridicon in Calypso, the SVG <title> (in this case gridicons-cross) is shown as a tooltip in Chrome.

https://raw.githubusercontent.com/Automattic/gridicons/master/svg/gridicons-cross.svg

Should we omit the title element where it just reflects the icon name?

2017-05-19 16_39_37

@folletto
Copy link
Contributor

I'm not even sure it makes sense to include a title now that you point it out. Afaik it's used for accessibility reasons, but this is an abstract library, and as such:

  1. If used in a decorative way, they probably shouldn't show up as interactive
  2. If used in an active way, they should have a title that is "noun+verb" of the action to be performed, not the iconography of the item

Either way, it seems something that the app using gridicons should be doing, not gridicons themselves.

Unless I'm missing something, clearly.

@davewhitley
Copy link
Contributor

davewhitley commented May 19, 2017

Seems like a bug. You can see that the title of the icons are formatted correctly in the svg-min folder. We need to keep the title for accessibility.

https://github.com/Automattic/gridicons/blob/master/svg-min/gridicons-cross.svg?short_path=b5df1b0#L1

For example, title of this icon is Cross not gridicons-cross.

So either the Gridicons package was not updated in Calypso, or Calypso is injecting its own title?

That being said, it's probably not a helpful title for a screen reader. Should be Close.

@mtias
Copy link
Member

mtias commented May 19, 2017

We need to keep the title for accessibility.

This was raised for dashicons, and the argument seems to be that it's the opposite, titles should be removed from the svgs: WordPress/gutenberg#528

The icon itself doesn't have a meaning, it's when it's wrapped as a button, etc, that it takes one, and then it's the button the one that should have an aria-role for the purpose of a12y.

@folletto
Copy link
Contributor

folletto commented May 19, 2017

Ok let's split the two discussion for clarity :)

Accessibility

Dave and I discussed a bit to clarify the positions above, and we both think some extra clarity is needed here: on one hand, title exists explicitly to be accessible, so it's good to have. On the other hand, "decorative images" and "images as part of a link" should have the alternative text void (see the WAI tutorial here).

I'd personally lean more in not including title, and @mtias link seems confirming it. I'd like to have @jasmussen as he suggested it in #186 for cross-check.

Minified or Not?

The two sources linked above are different: Dave's contain the right name, and it's from svg-min, Chri's doesn't, and it's from svg. In fact, we change it when we minify, as done in #189.

However, the React Component seems using the not-minified version. Why?

Update, it seems using the minified version tho: source.

@davewhitley
Copy link
Contributor

I just talked with Davide about it. Sounds like we need to remove them entirely. Looks like we can remove title in the svg min process.

@davewhitley
Copy link
Contributor

Oof, the svg sprite has the wrong titles. 😞

Maybe that's the problem?

@davewhitley
Copy link
Contributor

fixed by #207

all titles have been removed

@afercia
Copy link

afercia commented Jul 12, 2017

To clarify and for history:

This was raised for dashicons, and the argument seems to be that it's the opposite, titles should be removed from the svgs: WordPress/gutenberg#528

Theoretically, the SVG <title> and <description> elements are designed for accessibility, and should have meaningful values to give an accessible name and an accessible description. If that only worked.

Unfortunately, current support is not ideal. There are workarounds, see:
https://www.paciellogroup.com/blog/2013/12/using-aria-enhance-svg-accessibility/

However, implementing the technique described in the Paciello Group post is something hard to do programmatically. That's the reason why in Gutenberg we're trying to completely "silence" the SVG icons and use an aria-label on the wrapping element instead.

@folletto
Copy link
Contributor

Thanks @afercia. In Calypso we also tend to never use the icon alone, always wrapped in a button or some other kind of control – so I think we're in the same situation. :)

@davewhitley
Copy link
Contributor

@afercia thank you for the clarification! In that case, do you think #215 is a good idea for an icon set?

@afercia
Copy link

afercia commented Jul 13, 2017

@drw158 seems a sensible idea, not sure how SVGs can be programmatically built "on the fly" with and without title/desc though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants