ScottLogic / blog

The Scott Logic blog
http://blog.scottlogic.com
Other
9 stars 86 forks source link

Use landmark roles to improve navigation for screen reader users #138

Closed jcarstairs-scottlogic closed 6 months ago

jcarstairs-scottlogic commented 10 months ago

Some screen reader users use landmark roles to quickly navigate around webpages. We can use landmarks in our blog to make navigation easier for these users.

In particular, this PR:

ColinEberhardt commented 10 months ago

Can you please link to a published version of the blog with these changes so that I can check them visually?

jcarstairs-scottlogic commented 10 months ago

Can you please link to a published version of the blog with these changes so that I can check them visually?

https://ithake.github.io/blog-slogic/

It should be visually indistinguisable ;) the difference I'm hoping to make is that by exposing landmark roles, it will be easier for screen reader users to navigate through each page. I'm pressing d with NVDA.

ithake commented 10 months ago

It should be visually indistinguisable ;) the difference I'm hoping to make is that by exposing landmark roles, it will be easier for screen reader users to navigate through each page. I'm pressing d with NVDA.

@jcarstairs-scottlogic - some thoughts to consider for style and content: On the majority of projects, you'll be submitting PRs with attached unit tests, with the expectation that they'll be tested by QA specialist either manually or via existing test suite. Here, there's no expectation of an external tester, so there's more need for you to show your what testing you've done.

For example - https://github.com/ScottLogic/blog/pull/124 - I have screen shots before and after a functional change and a reference to doing (admittedly inconclusive!) non-functional testing for a non-functional change. It doesn't "prove" I've tested everything, but at least it shows I've tested something, and there some information there for the reviewer to pick up on - can they spot something else I should be testing?

"It should be visually indistinguisable ;)" in itself, this is light-hearted, but when you follow up with "the difference I'm hoping .. " the cumulative effect is that the reader wonders just how much uncertainty is involved here?

ithake commented 10 months ago

,,, by exposing landmark roles, it will be easier for screen reader users to navigate through each page. I'm pressing d with NVDA.

This all belongs in the introduction/description of the change.

Some readers use landmark roles to navigate.... Currently when you read blog site using NVDA (example screen reader) [this happens].

After making the change, when testing locally [something else happens].

Bonus points for:

The changes have been made in common include files so will effect all posts equally. I've tested using the list of representative posts that were agreed with our project sponsor previously.

Finally, your links to external documents in the PR description are really helpful, well done. If they're already detailed in a separate "Issues" document, then you don't need to repeat it all, instead, add a link to the document on sharepoint, and either ensure that it's publicly available or explicitly given the reviewer access to it as well.

ithake commented 10 months ago

+1 for the semantic structure!

Is it worth adding a main element to _layouts/default.html as well? On one hand it's not used AFAIK, on the other, it means we consistently have all the .html files in the _layouts folder defining a main for consistency.

jcarstairs-scottlogic commented 10 months ago

Regarding the failing pipeline: I think we're good to go.

I've looked through all the failures picked up by the pipeline on this PR. Almost all of them are colour contrast. The only other one is a <select> without an accessible name -- this is not new, and is fixed by #141.

It is unexpected that it picks up colour contrast issues: these should be ignored. I can't figure out why they're not being ignored on this PR. But it looks like when you make a new branch from gh-pages now, the problem goes away: see my dummy blog post PR. It's maybe something to do with the finer details of Git. I'll keep my fingers crossed and an eye on incoming PRs.