Skip to content

Conversation

@chenglou
Copy link
Contributor

This makes the headers red because they're links now.
Change them back to black.

@chenglou
Copy link
Contributor Author

Closed #290 a while ago because I thought the headers were clickable, just like Github READMEs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can just write #484848 here.

@chenglou
Copy link
Contributor Author

Thanks, isolated it in a variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it needs to be &:hover. I'm actually surprised sass didn't complain when trying to compile this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably :hover is the same as *:hover?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the current version generates

.documentationContent a :hover {
  text-decoration: underline;
}

Yours is:

.documentationContent a:hover {
  text-decoration: underline;
}

Which means for & to work I need to add color: $darkTextColor; to the &:hover too. Which one do you prefer?
Edit: I think @spicyj is right, in that case I'll change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe restructuring the html is actually best here - <h1><a id="foo" href="#foo">header</a></h1> - that also helps my concern about putting block level elements inside inline elements without making the <a> display: block.

And then we can do the css like:

h1, h2, etc {
  a {
    color: $darkTextColor;

    &:hover {
      text-decoration: underline;
    }
  }
}

Edit: Also, you'd only need to change the &:hover to include color if we are already setting color in a global a:hover rule, otherwise it should just inherit.

@chenglou
Copy link
Contributor Author

@zpao @spicyj there we go. Thanks for the tip. I still put id on h rather than a, because or else the page jumps to exactly below the a if you see what I mean.

This makes the headers red because they're links now.
Change them back to black.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants