search

mdn / content

The content behind MDN Web Docs
https://developer.mozilla.org
Other
8.88k stars 22.43k forks source link

Heading hierarchy #33298

Closed Crn0 closed 2 weeks ago

Crn0 commented 3 weeks ago

Description

change h4 to h2

Motivation

According to the mdn docs usage notes of the heading elements: h1 should be followed by h2.

Additional details

Related issues and pull requests

github-actions[bot] commented 3 weeks ago

Preview URLs

(comment last updated: 2024-04-30 05:25:33)

hamishwillee commented 3 weeks ago

There is no doubt that it would be good to fix this, but this is only a small part of the job. We would also need:

Can you do that?

If not, we should close this and create an issue to fix at some later point because while it is not "good" this is not at all relevant to the point of the tutorial, and unlikely to be teaching people bad form. So yes, would like fixed, but not a priority vs all the other things we might be doing.

Crn0 commented 3 weeks ago

Possibly scaling of the new heading to match the image via CSS update to the image of what it would look like when rendered https://pr33298.content.dev.mdn.mozit.cloud/en-US/docs/Learn/Server-side/Express_Nodejs/Displaying_data/Genre_detail_page#what_does_it_look_like

I suggest using inline CSS to scale the new heading and make it resemble an h4, rather than replacing the images. Would that approach be acceptable?

Every other template that uses this same pattern

The other details page? Yes, that makes sense. I will also handle it.

The example tutorial https://github.com/mdn/express-locallibrary-tutorial needs to be updated to match

I can also handle it, but it will require a separate pull request, correct?

teoli2003 commented 3 weeks ago

(Sorry I didn't mean to approve this one – Just ignore me)

hamishwillee commented 3 weeks ago

@Crn0 Yes, scaling the heading would be acceptable, and yes, we'd need a separate PR for https://github.com/mdn/express-locallibrary-tutorial, I would actually start with that, because then you can check that everything works before updating the content to match.

Thanks very much!

hamishwillee commented 3 weeks ago

Thank you. I'll try look at this and https://github.com/mdn/express-locallibrary-tutorial/pull/277 on Friday or the following week.