elixirschool / school_house

The new era of Elixir School now powered by @phoenixframework
https://elixirschool.com/
Apache License 2.0
155 stars 49 forks source link

Fix some sections lack heading #179

Closed sujinleeme closed 3 years ago

sujinleeme commented 3 years ago

fixes #176

jaimeiniesta commented 3 years ago

LGTM but have you checked the HTML generated? One way is using Rocket Validator with ngrok (see "validating your local server").

Another option is viewing the HTML source and pasting it in https://validator.w3.org/nu/#textarea

We want to check there are no issues related to headings.

sujinleeme commented 3 years ago

@jaimeiniesta Sorry, I hadn't tested validation correctly in 667cb20. Thanks for reminding me again.

In the _pagination.html, there is no heading tag inside the <section> container. I changed this wrapper tag to div in 29d7dbf.

When I check rocketvalidator, I can see all warning messages disappear now. https://rocketvalidator.com/s/f8bc533b-9ff3-4110-abac-2b2412e9c9ed

On the other hand, there is another h1 warning here. : https://rocketvalidator.com/s/f8bc533b-9ff3-4110-abac-2b2412e9c9ed/i/115772919

I would like to do HTML warning troubleshooting. :)

jaimeiniesta commented 3 years ago

Hey, that's great @sujinleeme - converting that section used in pagination on a div makes sense to me, as pagination is not content, that's why it doesn't need a heading.

Let's see if we can get this left h1 warning out of the way.

jaimeiniesta commented 3 years ago

I've checked your branch and on a quick look, I'm not sure about the <h1> warning.

I think we can leave this for another PR later on, if you're OK with the previous changes let's merge this so we can fix #176

sujinleeme commented 3 years ago

@jaimeiniesta No problem. I think there is some misusage of <h2> in page/index.html.heex.

https://github.com/elixirschool/school_house/blob/29d7dbfe803c6f486b189de173bcae9fecc5afbc/lib/school_house_web/templates/page/index.html.heex#L33-L38

I'd like to carry on this problem. Happy to learn HTML validation. :)

jaimeiniesta commented 3 years ago

Merged and deployed! Here's the HTML report for the site, let's see what's left :)

https://rocketvalidator.com/s/b2d48042-8061-4da0-922b-c4acd6e72328

Thanks @sujinleeme 🚀

jaimeiniesta commented 3 years ago
Captura de pantalla 2021-10-30 a las 12 53 26

Yay! After this fix, we're at a 94% of valid web pages regarding HTML issues 💪