Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

And some more classes have changed in the header #62

Merged
merged 4 commits into from
Aug 29, 2017

Conversation

joeytwiddle
Copy link
Contributor

@joeytwiddle joeytwiddle commented Aug 23, 2017

Today:

  • The .header class turned into .Header. (But in this PR I actually ignore the class and target the header tag instead.)

  • The .header-navlink which we got a few weeks ago has now become .HeaderNavlink

  • The notification indicator (bell icon) has become light grey. So when we make the header white, I make the indicator classic blue dark grey.

Users of the Chrome extension are currently getting this error: Cannot read property 'classList' of null and none of the greatness.

@joeytwiddle joeytwiddle mentioned this pull request Aug 23, 2017
src/header.css Outdated
@@ -19,6 +19,10 @@
color: #555
}

.great-header .notification-indicator {
color: #4078c0;
Copy link

@freewil freewil Aug 24, 2017

Choose a reason for hiding this comment

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

I think this should be #555 or a similar neutral gray to match the other icons. There is a separate blue icon if you have any notifications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I did wonder about that when I wrote it.

I have now styled it #333 and blue on hover, just like the other links in the header and the + icon beside it.

@DennisSnijder DennisSnijder merged commit efaf9fb into DennisSnijder:master Aug 29, 2017
@DennisSnijder DennisSnijder mentioned this pull request Aug 29, 2017
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