-
-
Notifications
You must be signed in to change notification settings - Fork 914
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
Conversation
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 So Capitalisation should stay the same as the brand. That's mostly Title Case. But weirdos like 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 What do you think? |
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. |
There was a problem hiding this 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.
images/svg/auth0.svg
Outdated
@@ -1,5 +1,5 @@ | |||
<svg xmlns="http://www.w3.org/2000/svg" | |||
aria-label="auth0" role="img" | |||
aria-label="Auth Zero" role="img" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be Auth0
.
images/svg/imgur.svg
Outdated
@@ -1,6 +1,6 @@ | |||
<svg xmlns="http://www.w3.org/2000/svg" | |||
aria-label="imgur" role="img" | |||
aria-label="Imgur" role="img" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They style it lowercase.
images/svg/w3c.svg
Outdated
@@ -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" |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep lowercase.
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.