Skip to content

Utilising semantic tags to provide better structure for screen readers#1464

Open
smallsaucepan wants to merge 3 commits intoMLTSHP:mainfrom
smallsaucepan:macro-semantic-tags
Open

Utilising semantic tags to provide better structure for screen readers#1464
smallsaucepan wants to merge 3 commits intoMLTSHP:mainfrom
smallsaucepan:macro-semantic-tags

Conversation

@smallsaucepan
Copy link
Contributor

Overview

Utilising semantic tags (main, header, article, nav, etc) instead of divs and reordering content in HTML to header, "content", aside, footer to provide better structure for screen readers. Subset of intended changes seeking feedback on approach. Will make further commits based on feedback.

Screenshots

Only visual change should be to alignment of Older and Newer pagination buttons on mobile. They have been moved closer to edge of screen and should line up with edge of image content.

Screenshot 2026-03-08 at 13 19 00

Testing

Have only done a subset of screens initially. Seeking feedback before converting them all. Main ones done are:

  • Base
    • Layouts
  • Components
    • Image
      • List View
      • Lengthy Content
      • Comments Open
    • Pagination
  • Areas
    • FAQ
    • Find Shakes
    • Incoming
    • Legal
    • Relationships
    • Settings
    • Shake

Pages should look and behave as they currently do, albeit with different behind the scenes markup.

Some design decisions worth noting:

  1. Previously a list of image posts consisted of alternating header and content sibling divs. Each post (header and content) has been wrapped in an article tag to identify it as self contained content.
  2. With paginated results the list being paginated no longer displays a trailing divider (assumes there will be pagination). Instead the pagination control takes responsibility for placing a preceding divider.

…ontent in HTML to header, "content", aside, footer to provide better structure for screen readers. Subset of intended changes seeking feedback on approach.
@smallsaucepan smallsaucepan requested a review from spaceninja March 8, 2026 02:26
Copy link
Member

@spaceninja spaceninja left a comment

Choose a reason for hiding this comment

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

Heya! Just wanna say that overall I think this is a good change, and I'm happy to see it.

That said, you've made two changes at the same time that make this PR difficult to review — You've made simple markup changes (div to main, followers-list class, updating some profiles, etc), and you've reordered some markup (moving the main body content first, with the sidebar following). I like these both, but when they both happen in the same file, the diff becomes useless, because there's just one huge chunk of changes from the order change, and any other changes are hidden.

Can you tease these apart and make one PR with the order changes and another with the markup changes? I'm happy to approve them both, but for the PR where you're moving the code around, please don't make any other changes, so the review is simply "this block of code moved down the page"

… sidebar to end of file) to allow for easier diffing of this PR. Will re-apply in a subsequent PR.
@smallsaucepan
Copy link
Contributor Author

Easy done. Have rolled back the reordering changes to be reapplied in a subsequent PR.

Copy link
Member

@spaceninja spaceninja left a comment

Choose a reason for hiding this comment

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

Thank you for breaking this apart, it was much easier to review.

This is looking good! A few nitpicks and then I'm happy to merge.

Also, could you add an UNRELEASED section to the top of the changelog file listing all the breaking markup changes that will need to be applied in the MLTSHP app? (Or you can do that in the followup PR, if that makes more sense, but it'll need to be done before we bump the version)

<a class="btn btn-secondary btn-shadow" href="#">Newer »</a>
</div>
</div>
<nav class="linear-navigation" aria-label="Timeline">
Copy link
Member

Choose a reason for hiding this comment

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

Why is this the only one with an aria-label?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the most obvious one to me where a label would help. Not meant to be an exhaustive list though - just an example and conversation starter for this PR.

}

> * {
min-width: 0; // https://github.com/philipwalton/flexbugs/issues/39
Copy link
Member

Choose a reason for hiding this comment

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

Did you confirm that word-wrap works properly in flex containers in Firefox now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, will probably have to add this back in. However existing selector was causing problems with restructured HTML as it is.

<a class="older btn btn-secondary btn-shadow" href="#">« Older</a>
</nav>

<hr />
Copy link
Member

Choose a reason for hiding this comment

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

Let's kill the HRs there, they're not adding any value on the style guide page (since the nav item itself has a top border already)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@smallsaucepan
Copy link
Contributor Author

Have addressed the feedback as best I can for now. Should I proceed like so from here?

  1. finalise any remaining semantic tag changes (minus section reordering) and commit to this PR
  2. submit reordered sections as a second PR
  3. submit odds and ends (like word-wrap in flex containers) as a third PR

Suspect there will be things to tidy up after reordering, hence third PR.

@spaceninja
Copy link
Member

Sounds like a great plan! Ping me when you're ready for another review on this PR. 🎉

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