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

Added Aria-Label to image links in index page for improved accessibility #548

Closed
wants to merge 2 commits into from

Conversation

@bmitchinson
Copy link
Contributor

commented May 31, 2019

Changes proposed

Inserting the relative post title as a string in Aria-Label tags and image alt parameters for redirects. This is an accessibility standard best documented here.

Including this change bumped my Lighthouse audit accessibility score by 15, meaning tranquilpeak now scores above 90 by default when using the theme. I feel anything we can include in the theme itself to increase accessibility is a plus.


This change is Reviewable

@bmitchinson bmitchinson changed the title Added Aria-Label to image links in index for improved accessibility Added Aria-Label to image links in index page for improved accessibility May 31, 2019

@fletchto99

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

I also noticed it fails the contrast tests. Did you want to have a run at that or shall I?

@fletchto99

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

We should also do this for links within the post as well as the sidebar links :)

@bmitchinson

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

I don't plan on doing much more, you're welcome to pick it up. What's your planned idea for the contrast tests? My understanding was that it ruled grey on white was inaccessible. I was conflicted though because I like the grey on white :(

@LouisBarranqueiro
Copy link
Owner

left a comment

👋 @bmitchinson good idea. 👍 Could you please post the benchmark before and after so we can measure the benefits of these changes?

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @bmitchinson)


layout/_partial/index.ejs, line 34 at r1 (raw file):

        <div class="postShorten-wrap">
            <% if (thumbnailImageUrl !== null && thumbnailImagePosition === "top") { %>
                <a href="<%- url_for(post.path) %>" aria-label="<%= post.title %>">

aria-label is used to describe the current element. This is not really a correct label for this link. I suggest aria-label="Open the article: <%= post.title %>"


layout/_partial/index.ejs, line 36 at r1 (raw file):

                <a href="<%- url_for(post.path) %>" aria-label="<%= post.title %>">
                    <div class="postShorten-thumbnailimg">
                        <img alt="<%= post.title %>" src="<%= thumbnailImageUrl %>"/>

same remark. I suggest alt="Thumbnail image of the article: <%= post.title %>"

@LouisBarranqueiro LouisBarranqueiro self-assigned this Jun 9, 2019

@bmitchinson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

I'm unavailable to post an accurate benchmark, as I don't want to revert my site in order to get a proper live measurement. If you'd like to compare an existing sites post, with one of mine, maybe that would be accurate? Just updated the branch with your request.

@LouisBarranqueiro

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2019

You can do it with the demo site. It's here for that. You can find the source of the posts here: https://github.com/LouisBarranqueiro/hexo-theme-tranquilpeak/tree/source-blog-posts , then you just need to setup a new blog.

I'm gonna give it a try to see the results

@LouisBarranqueiro

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2019

I just did a test, and these changes have no effect on the demo site. The score before and after is 84. The reason is that you've only added labels for a type of link. This job needs to be done on the entire page, and in fact on the entire site to do things right and have a better score.

Also, it won't be useful for chinese or french users because these labels are in English. We need to translate those ones in order to do the job correctly.

Would you be able to do that?

@bmitchinson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 16, 2019

I'll take a look. When you say "those labels are in English", would the ideal solution be to dynamically switch them based on the language? Would I just put them into translations files?

@LouisBarranqueiro

This comment has been minimized.

Copy link
Owner

commented Jun 21, 2019

There is quite some work to do it for the entire theme so I will take care of this. Thanks again for the proposition, and as always, I really appreciate your motivation 😄 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.