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

Check for correct order of heading tags #13

Closed
SavvasStephanides opened this issue Sep 25, 2020 · 14 comments · Fixed by #82
Closed

Check for correct order of heading tags #13

SavvasStephanides opened this issue Sep 25, 2020 · 14 comments · Fixed by #82
Assignees
Labels
a11y feature New feature or request for an a11y check

Comments

@SavvasStephanides
Copy link
Contributor

Headings should be in the correct order See here for more information.

Skipping heading ranks can be confusing and should be avoided where possible

For example, this is correct:

<h1>This is a heading</h1>
<h2>This is a subheading</h2>

This is incorrect because it goes from h1 straight to h3 (skipping h2):

<h1>This is a heading</h1>
<h3>This is an h3 heading</h3>

Not completely sure how this can be done with CSS. I have tried this

h1+h3,
h1+h4 {
    border: var(--checka11y-error-border-width, 5px) solid var(--checka11y-error-color, red) !important;
}

which checks if an h1 heading is followed by an h3 or h4 header. However, this doesn't take into account elements in between the headers. Any ideas welcome.

@jackdomleo7
Copy link
Owner

I like this idea, but your last point was main concern. It's more common for headings to not be in the same scope. Let me consult @danspratling on ideas for this too. 🤔

@jackdomleo7 jackdomleo7 added a11y feature New feature or request for an a11y check needs more info Further information from author is requested labels Sep 25, 2020
@danspratling
Copy link
Contributor

danspratling commented Sep 25, 2020

You can use the ~ selector to select "any elements after".

Thinking about how you'd apply this you could do something like

h1 ~ h3 {
  // Error
}

h1 ~ h2 ~ h3 {
  // Remove error
}

Without testing this in actual code though I'm not sure if this is robust enough. Maybe it is, as once an h2 is present an h3 can be placed after it multiple times if required, which this should evaluate correctly.

@jackdomleo7
Copy link
Owner

Hmm interesting. @SavvasStephanides if you'd like to, feel free to experiment this and see what you can find out? 😊 Or we can pick it up at some point.

@danspratling
Copy link
Contributor

Here's a pen I created quickly which covers how this should work. Seems robust enough to me

https://codepen.io/dan_spratling/pen/xxVBxgK

@alvaromontoro
Copy link
Contributor

That solution could be improved –correct me if I'm wrong– by adding the universal operator in this way:

h2 ~ h1,
h2 ~ * h1,
h3 ~ h2,
h3 ~ * h2,
...

Although it may not be too efficient, it will capture more cases as it considers a more realistic use of the headings in HTML (e.g., headings are rarely at the same level, they are normally within their own container/section.)

Here is a codepen with a demo: https://codepen.io/alvaromontoro/pen/OJNeJwX

@alvaromontoro
Copy link
Contributor

This could potentially be integrated with something similar to what Una Kravets suggested once on Twitter (look at the second tweet as it's more accurate). Her solution targets a slightly different thing (but both cases highlight errors that need to be fixed.)

@danspratling
Copy link
Contributor

danspratling commented Oct 1, 2020

Edit: Just read through all this again and I definitely need to stop looking at issues when I'm sleepy!

Both of your suggestions are right. Would you be happy to create a PR incorporating them?

@danspratling danspratling added the Hacktoberfest Hacktoberfest eligible label Oct 1, 2020
@SavvasStephanides
Copy link
Contributor Author

SavvasStephanides commented Oct 1, 2020

@alvaromontoro Thanks for this. I don't know if I'm missing something here but it doesn't seem to catch it when a h4 is defined after a h2 when I test it.

@alvaromontoro
Copy link
Contributor

@SavvasStephanides That's why I mentioned that both solutions targeted different things:

  • The one that you suggested works great at searching skipped headings in an incremental way (e.g., when you skip from h2 to h4)
  • Una's approach is in the other direction looking for descendant headings of a higher level (e.g., when you have an h1 inside an h2)

They can be combined and they'll highlight wrong headings correctly. The only "issue" is that sometimes they will highlight both sides. E.g., if you have h1-h3-h2, your method will highlight h3, Una's will highlight h2, combining both will highlight h3 and h2... I don't even know if that can be considered an issue at all.

@jackdomleo7 jackdomleo7 removed the Hacktoberfest Hacktoberfest eligible label Nov 9, 2020
@jackdomleo7
Copy link
Owner

@alvaromontoro are there any flaws in your proposed CodePen? https://codepen.io/alvaromontoro/pen/OJNeJwX

@alvaromontoro
Copy link
Contributor

@jackdomleo7 I think combining both methods like in the demo works fine. If you want I can pick this ticket (if it's not pre-assigned to anyone)

@jackdomleo7
Copy link
Owner

@alvaromontoro only if you want to, that'd be great! 😊 If not, don't worry, I was just gathering any missing information.

@alvaromontoro
Copy link
Contributor

Assign it to me then. I'll work on it.

@jackdomleo7 jackdomleo7 removed the needs more info Further information from author is requested label Nov 16, 2020
@alvaromontoro
Copy link
Contributor

Found a bug in the demo... Need to add all the possible combinations of * and no-*. The rule is big and bugly, but now it seems to work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y feature New feature or request for an a11y check
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants