-
Notifications
You must be signed in to change notification settings - Fork 59.9k
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
Inconsistent formatting of text with octicons #35326
Comments
@akordowski Thank you for raising this issue! I'll get this triaged for review ✨ Our team will provide feedback regarding the best next steps for this issue - thanks for your patience! 💛 |
Hi @akordowski—many thanks for offering to open a PR for this and for opening an issue first. To your first point: you're correct that this is a discrepancy. Did you run a script to determine how many times one or the other occurs? I'm not aware that this causes any problems with the end user experience with the docs, but in principle, I have no problem opening that up to you to contribute. However, please see my points below:
If all this sounds OK, I'm happy to open it to you to contribute. |
@subatoi Thank you for your response.
No, I did a simple text search.
No, it does not and mostly users may not notice it at all 😉 But IMHO a documentation should use a consistent style.
No problem.
There are only 22 results of using the |
@akordowski No problem, thank you for your help! We really appreciate your contributions.
I think the bigger picture is that we want to make things as accessible as possible for screen readers, but I'm happy to loop in @nguyenalex836 who can liaise internally with relevant stakeholders and decide on the best way forward for that particular point. If you prefer to wait for the outcome of Alex's research into that before going ahead with any changes, that's fine ... or else if you prefer, you're welcome to start opening PRs now that are limited to changing the placement of |
@subatoi As requested I provided 4 PRs for the changes addressed in this issue.
Yes, of course.
If this is the case then it would made sense to provide a |
That's great! We'll review those as soon as we can. Thank you so much!
I would agree. I'll let @nguyenalex836 follow up on this point and we'll come back to you separately 👍 |
@akordowski @subatoi Thank you both for working on those initial PRs! ✨
I'll be reaching out to our SME teams for guidance on this 💛 stay tuned! |
Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert 👀 |
@akordowski Hello! 👋 Our SMEs responded with the following when asked about inconsistency regarding our usage of
Given the above, I'll go ahead and close this issue as completed since all the linked PRs have been merged 💛 Thank you again for all of your work on this front! ✨ |
@nguyenalex836 Thanks for the response. But there are also cases where there is an icon AND text, as you can see below: Lines 209 to 212 in 17af282
For such cases it would make sense to replace the |
@akordowski No problem at all! Let me check with our SMEs on that scenario, I'll return with more info soon 💛 |
@akordowski Our team agrees with you! 💛
Feel free to open PRs for this whenever you have time - thank you! ✨ |
@nguyenalex836 Thank you, will do tomorrow. |
Code of Conduct
What article on docs.github.com is affected?
Multiple articles.
What part(s) of the article would you like to see updated?
I noticed that the formatting of text with octicons is in the docs inconsistent:
EDIT:
There is also the usage of
aria-label
instead ofaria-hidden="true"
:The formatting should be unified to:
Additional information
I could provide a PR to fix this issues.
The text was updated successfully, but these errors were encountered: