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 h1 warning #182

Closed sujinleeme closed 3 years ago

sujinleeme commented 3 years ago

This PR is to fixh1 warnings in #181.

Before https://rocketvalidator.com/s/41643d18-3a3f-4c34-85f7-fbdb1525636f/i?search=heading&page=1&non_doc=false

After changes https://rocketvalidator.com/s/39d99f7a-b786-4c3b-a3d6-11783bdceb85/i?search=heading&page=0

jaimeiniesta commented 3 years ago

Hi @sujinleeme - this does indeed remove the h1 warnings but I think that heading structure in the home page is wrong, for example there are 2 h1s, one for the "Welcome" at the top and one for the "Recent posts" at the bottom. There should be only one h1, for the main section.

You can use the HeadingsMap extension to get an outline of the headings structure, for example:

Captura de pantalla 2021-11-02 a las 10 14 51

You can see this heading structure is a mess and doesn't correspond to the layout we see on the screen.

I think it would make more sense to have only these headings:

h1: Welcome to Elixir School!
  h2: Why Elixir?
  h2: By the Numbers
  h2: Recent Posts

Then, within "Why Elixir?", I would not use headings for the sub-sections "Functional Programming", "Feature Packed", as the content is so short. I think that a p with bold font style is enough. If headings are used, then it needs to be an h3 because it's nested under an h2.

Lastly, the footer also uses headings but I think it shouldn't use them for the same reasons, but we can leave the footer for later.

sujinleeme commented 3 years ago

@jaimeiniesta I had the same idea. That's a nice suggestion!

I addressed your comment so it would have this tree. : Screenshot 2021-11-02 at 13 07 31

I changed <p>classes in lib/school_house_web/templates/post/index.html.heex following other sections' stylings.

https://github.com/elixirschool/school_house/pull/182/commits/cf25c8720ca6c7724abbe36d2cc3069126f1e3fb#

I will fix the footer structure in different PR. Thanks for your kindness! 🤠

sujinleeme commented 3 years ago

@doomspork Could you taghacktoberfest and hacktoberfest-accepted label? Thanks!