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

footer.html: replace h5 with a span element #460

Closed
wants to merge 3 commits into from
Closed

Conversation

XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Apr 1, 2024

Preview: https://deploy-preview-460--bootstrapblog.netlify.app/

Should be backported to icons and main repo

@XhmikosR XhmikosR marked this pull request as ready for review April 1, 2024 14:52
@XhmikosR XhmikosR marked this pull request as draft April 1, 2024 14:58
@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 1, 2024

@patrickhlauke does it LGTY?

Unsure what's the best approach here, previously our headers didn't follow the order.

@patrickhlauke
Copy link
Member

don't really agree with the PR here. that text definitely acts as a heading for the list that follows, so having it marked up as a heading makes semantic sense. <h5> is fine (as skipping heading levels is not a necessarily a failure)

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 2, 2024

Cool, dropping it then, thanks!

FYI I was trying to fix the Lighthouse a11y issues and this was one of them. Would be nice if someone could have a look at those.

@XhmikosR XhmikosR closed this Apr 2, 2024
@XhmikosR XhmikosR deleted the xmr/footer-h5 branch April 2, 2024 05:59
@patrickhlauke
Copy link
Member

overall, the heading structure on the blog is a tiny bit quirky, with the <h1> for "The Bootstrap Blog", and then every blog post itself starting with an <h1>... but it's excusable.

one way to bring a bit more order into the structure (and to sort out the heading levels a bit) would be to add a hidden <h1> at the start of the footer, and then have those <h5>s set to be <h2>s (but visually styled to be the same as now)

(for the other lighthouse issue, I'd suggest making the links in that footer text bold or underlines, to provide another visual hint other than just their colour that these are links)

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 2, 2024

IIRC we get the same Lighthouse errors on the main site too, that's why I decided to try to fix the problem here and then backport the changes

@julien-deramond
Copy link
Member

julien-deramond commented Apr 2, 2024

You could probably reopen this PR and replace your <span ... with a <h2 class="h5"> that would bring the right semantics with the same rendering as before. And, as mentioned by Patrick, add a hidden <h1> for the footer.

@XhmikosR
Copy link
Member Author

XhmikosR commented Apr 2, 2024 via email

@patrickhlauke
Copy link
Member

Is it really considered a good approach?

because there's no way to effectively "reset" the context of headings (since the whole "outline algorithm" that was proposed for HTML years ago is a pipe dream that was never implemented), just having them as <h5> at the moment without anything else before them makes them hierarchically come under the last blog post. While not the most elegant, having a hidden <h1>Footer</h1> or <h1>Additional links</h1> or similar - or even wrapping the bootstrap link at the start of the footer there in an <h1> - is probably the best we can do to make it clear those headings that then follow are "outside" of the last blog post

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.

3 participants