Use headers correctly

[casebenton]

Only look at most recent commit, as other two commits are included in another PR currently.

@mythmon r?

[mythmon]

So lets talk about semantics. <h3> is "third level heading". <p> is "paragraph". This doesn't matter a lot for layout and getting the visual affect you matter. Where it does matter for readability of the code, maintainability by other people, and accessibility to users.

Some text above some other text that holds a title or subject is a heading. The highest level heading should be <h1>. <h2> would be used to introduce a sub-topic, <h3> to introduce a sub-sub topic, and so on to <h6>.

The body of a text, in between the headings, is generally arranged into paragraphs, and so would use <p>, one for each paragraph.

In this context, I would expect to see something like this:

<div>
  <h1 class="name">Add-on</h1>
  <p class="description">description of add-on</p>
</div>

At this point, the classes (name and description) seem a little redundant, if you read this with the semantic knowledge provided by <h1> and <p>. It still might be useful, but it might not. Of course, the default styling of <h1> is way to big and bold for your uses, but that is easily fixed with CSS.

[mythmon]

As a side note: I don't think any of the changes here in the third commit actually depend on the first two. You should be able to isolate that commit and make several unrelated branches (disclaimer: I haven't actually looked at the git history). I'd expect something like this to work:

$ git reset --hard master
$ git cherry-pick a7f269b
$ git push --force-with-lease

--force-with-lease is a safer version of --force that only forces the push if what you expected to be there is there. It is a way to avoid writing over changes made elsewhere, such as by another person or in the GitHub web editor. (In zsh, I auto complete it with git push -f-w<tab>. We might have different autocompletes though).

[casebenton]

Great! I was able to remove the extra two commits. I'll add another commit soon that uses h1 and p properly. I'll probably remove the classes as well.

[casebenton]

Ok so I think I'll actually leave the classes for now, as they're used in many query selectors in the content script. It seems like more of a headache than it's worth to remove them and make sure the script doesn't break

[mythmon]

You didn't actually change the HTML, just the CSS in that last commit.

[casebenton]

Oops. I think it's fixed now. :)

[mythmon]

This looks good now, r+.

You have a merge conflict though. A good way to fix this would be to merge master into this branch, push that, and then click the green button here.