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

Fix aria labels, linux rights, underscores #334

Merged
merged 5 commits into from
Oct 23, 2019
Merged

Fix aria labels, linux rights, underscores #334

merged 5 commits into from
Oct 23, 2019

Conversation

vangyyy
Copy link
Contributor

@vangyyy vangyyy commented Oct 18, 2019

Fix aria-labels

I ran all svg through grep with:

for file in *.svg; do echo $file; cat $file | grep "aria-label"; done

and fixed some mistakes in aria-labels, mainly Capital letters and some broken templates. I have a question though. Should aria-labels include spaces even in names that does not contain one?

Examples:

"JavaScript" or "Java Script"
"Auth0" or "Auth Zero"
"Pocketcasts" or "Pocket Casts"

Official name is of course JavaScript, but text-to-speech software may have problems reading that. Same way with Auth0, that may be even better example. So what is the right way ? I can correct all of the SVGs, but we should agree on this first.

Remove executable rights

Some files had executable rights so:

sudo chmod 644 ./images/svg/*.svg

Just a small thing for consistency.

Consistent naming scheme

I have renamed files that contained "-" to "_", so they are consistent.

Example:

"amazon-s3.svg" -> "amazon_s3.svg"

Only exception:

I left out "ko-fi.svg", since the "-" is part of official name, but it may be a good idea to change this also, just for the convenience and consistency.

Reasoning:

Android drawables can not contain "-" and have to be renamed, this makes things easier when generating them. Also would be a good idea to include it as a rule in contributions guidelines.

@edent
Copy link
Owner

edent commented Oct 21, 2019

This is excellent work - but I need to have a think about this.

In general, ARIA labels should match what a sighted person would see. So JavaScript. This gives it consistency of pronunciation with other text on the site.

So Auth0 will be pronounced the same in a label or running text. And Whatever CI should be written like that - not Whatever Sea Eye.

Capitalisation should stay the same as the brand. That's mostly Title Case. But weirdos like imgur insist on a lower-case starting character.

As for dashes.... I had no idea Android Drawables didn't use them! I don't know how many upstream people use STI (not many!) but I'm a bit wary of changing the existing SVG names.

So.... I think keep - in the SVG names, and transform them to _ for drawables.

What do you think?

@vangyyy
Copy link
Contributor Author

vangyyy commented Oct 21, 2019

Android drawables are already using '_' after my recent PR. The import script in andOTP for example takes care of it anyway, I just wanted to point that out. It's not that big of a problem.

Copy link
Owner

@edent edent left a comment

Choose a reason for hiding this comment

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

I've left 4 small requests. After that, it's good to merge.

@@ -1,5 +1,5 @@
<svg xmlns="http://www.w3.org/2000/svg"
aria-label="auth0" role="img"
aria-label="Auth Zero" role="img"
Copy link
Owner

Choose a reason for hiding this comment

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

Should be Auth0.

@@ -1,6 +1,6 @@
<svg xmlns="http://www.w3.org/2000/svg"
aria-label="imgur" role="img"
aria-label="Imgur" role="img"
Copy link
Owner

Choose a reason for hiding this comment

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

They style it lowercase.

@@ -1,7 +1,7 @@
<svg xmlns="http://www.w3.org/2000/svg"
aria-label="w 3 c" role="img"
aria-label="W 3 C" role="img"
Copy link
Owner

Choose a reason for hiding this comment

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

Should just be W3C. No spaces.

README.md Outdated
@@ -32,7 +32,7 @@ Say thanks!
<td>Pinterest<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/pinterest.svg" width="125" title="Pinterest" /><br>526 Bytes</td>
<td>VK<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/vk.svg" width="125" title="VK" /><br>534 Bytes</td>
<td>Mastodon<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/mastodon.svg" width="125" title="Mastodon" /><br>550 Bytes</td>
<td>imgur<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/imgur.svg" width="125" title="imgur" /><br>356 Bytes</td>
<td>Imgur<br><img src="https://edent.github.io/SuperTinyIcons/images/svg/imgur.svg" width="125" title="Imgur" /><br>357 Bytes</td>
Copy link
Owner

Choose a reason for hiding this comment

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

Keep lowercase.

@vangyyy vangyyy requested a review from edent October 23, 2019 10:42
@edent edent merged commit fd79fb4 into edent:master Oct 23, 2019
@vangyyy vangyyy deleted the fix-aria-rights-underscores branch October 25, 2019 11:01
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.

2 participants