search

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 element div not allowed as child of element h3 #173

Closed AshutoshSajan closed 3 years ago

AshutoshSajan commented 3 years ago

resolve #170

AshutoshSajan commented 3 years ago

Thanks but I'm not sure this would fix the issue, as a <p> tag isn't allowed inside an <h3> either.

Please ensure that your fix will generate valid HTML in the W3C Validator: https://validator.w3.org/nu/#textarea

okay, I will have a look. Thanks for the info.

AshutoshSajan commented 3 years ago

@jaimeiniesta is it a good idea to add span here? But I think adding span can also cause styling issues.

jaimeiniesta commented 3 years ago

Hi @AshutoshSajan - the issue is that we're using h3 for the "previous lesson" and "next lesson" links near the footer. These should not be contained within an h3, as these tags are for sections of the documents.

Check this screenshot, here I've used the HeadingsMap chrome extension to see the heading structure and we can see that we're creating some confusion about the document structure.

Captura de pantalla 2021-10-26 a las 9 25 50

Instead of using h3 for those links, we should just style them with CSS. Better still, we should declare some kind of navigation but for now let's just remove those h3 :)

jaimeiniesta commented 3 years ago

@AshutoshSajan here's what I would do to fix this issue, what do you think?

https://github.com/elixirschool/school_house/pull/175/files

Feel free to incorporate these changes in your PR :)

jaimeiniesta commented 3 years ago

Fixed in https://github.com/elixirschool/school_house/commit/a45b784da7a20fb2c929fcc1aabc3194915bfa3b

AshutoshSajan commented 3 years ago

@AshutoshSajan here's what I would do to fix this issue, what do you think?

https://github.com/elixirschool/school_house/pull/175/files

Feel free to incorporate these changes in your PR :)

Looking good 👍🏽, Sorry I was little busy