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

Notification drawer blank state styling wrong #604

Closed
spadgett opened this issue Aug 31, 2017 · 9 comments
Closed

Notification drawer blank state styling wrong #604

spadgett opened this issue Aug 31, 2017 · 9 comments
Assignees

Comments

@spadgett
Copy link
Contributor

Looks like this was introduced in #587

cc @jeff-phillips-18 @benjaminapetersen

Before as h1:

screen shot 2017-08-30 at 8 26 42 pm

After as h4:

screen shot 2017-08-30 at 8 26 57 pm

@spadgett
Copy link
Contributor Author

The original h1 seems more semantically correct to me. h4 seems arbitrary since there's no h1 / h2 / h3.

@benjaminapetersen
Copy link
Member

benjaminapetersen commented Aug 31, 2017

Hm. Yeah I wonder if semantically it could have just been a p tag as its not really even a heading for content.

The top header area (test, view all events, 0 unread) is within an h4, btw. Reported here

@benjaminapetersen
Copy link
Member

Running the demo locally looks like this:

screen shot 2017-09-01 at 2 46 13 pm

The h1 class just affects the heading space. However, when we pull it into the console, it blows up the font size.

I do see .showcase classes appearing here and altering style as well, however, which might be related to what @jeff-phillips-18 mentioned on issue #607?

@jeff-phillips-18
Copy link
Member

.showcase is a ng-docs page only thing. Just went in today. we will resolve it shortly.

@benjaminapetersen
Copy link
Member

ok great!

@spadgett
Copy link
Contributor Author

spadgett commented Sep 1, 2017

Yeah I wonder if semantically it could have just been a p tag as its not really even a heading for content.

Agree

The h1 class just affects the heading space. However, when we pull it into the console, it blows up the font size.

It looks like class="h1" in actually the problem in our console because of one of our styles

@cliffpyles
Copy link
Contributor

According to the design spec, an empty state pattern requires a title. So from what I can tell, the larger font and spacing is actually the desired outcome (according to the spec). So do we need to deviate from the design spec, or should this be left as is? I'm attaching the design spec below for reference.

image

@jeff-phillips-18
Copy link
Member

I think leaving it as an h4 should be OK, it is technically a header.

@cliffpyles
Copy link
Contributor

Sounds good. I'll close the issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants