Skip to content

Conversation

muffinresearch
Copy link
Contributor

@muffinresearch muffinresearch commented May 16, 2016

Fixes: mozilla/addons#9589

Before:
discover_add-ons

After:

discover_add-ons

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0b846b1 on muffinresearch:update-heading-css-to-spec into 70015f0 on mozilla:master.

@mstriemer
Copy link
Contributor

mstriemer commented May 16, 2016

I think the heading text looks nicer at 22px, the copy is definitely better though. Maybe we can leave the heading at 22px and @pwalm won't notice?

r+

@muffinresearch
Copy link
Contributor Author

I think let's do it to spec and ask UX if they want any changes when they look at the real thing in practice.

Fwiw: re: the h1 the headings in the mocks looked to have letter-spacing which I'd previously eye-balled so it's not just the font-size change here the letter-spacing is also removed back to the default.

@muffinresearch muffinresearch merged commit 2e08384 into mozilla:master May 16, 2016
@mstriemer
Copy link
Contributor

Ah, yeah that's what it is. It looks cramped to me now.

@pwalm
Copy link
Contributor

pwalm commented May 16, 2016

Let's stick with "After." The type looks more balanced. 👍

@muffinresearch muffinresearch deleted the update-heading-css-to-spec branch April 3, 2019 10:41
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.

Update header to spec
4 participants