mozilla / advocacy.mozilla.org

https://advocacy.mozilla.org
Mozilla Public License 2.0
203 stars 34 forks source link

Adding new PNI nav #504

Closed gvn closed 6 years ago

gvn commented 6 years ago

related to #497

gvn commented 6 years ago

@beccaklam Can you give this a design review?

https://advocacy-mozilla-org-st-pr-504.herokuapp.com/en-US/privacynotincluded/

beccaklam commented 6 years ago

Yes! @gvn sorry I saw it this morning but got distracted. Will get on it right away.

beccaklam commented 6 years ago

@gvn: So some quick feedback:

moz-logo-one-color-white-rgb

.buyers-guide-header-container { padding: 20px 45px; to padding: 20px 60px 24px }

and (actually not sure if this is the right selector but see image below)

.buyers-guide-home .categories-container .home-category-container .category-top { padding: 0 45px to padding: 0 60px }

screen shot 2018-01-29 at 1 04 09 pm
gvn commented 6 years ago

@beccaklam Ok, made some of those tweaks. I'm not sure what you mean by centering the menu content exactly.

Do you mean setting a max-width on the container or vertically centering it in the viewport? The comps look left aligned...

beccaklam commented 6 years ago

@gvn Oh sorry I just meant centering the content. Setting the div that holds menu content to

margin: auto 0;

seems to do it.

screen shot 2018-01-29 at 3 12 53 pm
beccaklam commented 6 years ago

^ also I am still seeing a black logo @gvn am I using a wrong staging link?

gvn commented 6 years ago

You may have an older version cached. Try holding shift and clicking refresh.

So this is really what you want? Looks odd to me:

screen shot 2018-01-29 at 1 05 33 pm
beccaklam commented 6 years ago

@gvn haha it's switched around:

margin: auto 0;

instead of

margin: 0 auto;

beccaklam commented 6 years ago

@gvn also shift+refresh did not work. it's also still black in your last screen cap? i'm talking about the Mozilla logo in the full screen menu

gvn commented 6 years ago

Ok, the filter wasn't working properly x-browser, but should be fixed now. I also tweaked the margin.

https://advocacy-mozilla-org-st-pr-504.herokuapp.com/en-US/privacynotincluded/

beccaklam commented 6 years ago

@gvn Mobile view is looking good but needs some tightening up: Current state: screen shot 2018-01-29 at 4 44 39 pm

gvn commented 6 years ago

Made those tweaks...

beccaklam commented 6 years ago

@gvn thank you! Sorry I found some more tweaks which relate more to ticket #495 I forgot to QA the mobile view which is my bad, this comment basically refers to the "tightening up for small screens" @alanmoo mentioned. So for all these changes I'm thinking they're for a media query that deals with any screen width less than 500px? (Also let me know if you would prefer this in another ticket)

beccaklam commented 6 years ago

@gvn One more menu design QA: Can we shift the X over to where I've drawn the red X in the following image:

screen shot 2018-01-30 at 12 04 19 pm
gvn commented 6 years ago

Ok, tweaks made. @beccaklam can you take another look at staging?

beccaklam commented 6 years ago

@gvn Just looked. Soooo much better. Sorry I made a mistake:

gvn commented 6 years ago

K...yeah, thought that looked weird.

beccaklam commented 6 years ago

@gvn and

After that we should be good to ship! Thank you so much đź’Ż

gvn commented 6 years ago

Ok, I removed the forced break between "privacy" and "not included".

I added an additional breakpoint below 400px to reduce to 12px because it was getting really crowded at 16px.

beccaklam commented 6 years ago

@gvn Design QA:

P1

Universal

P2

Universal

Mobile

Tablet/Desktop

P3

Mobile

gvn commented 6 years ago

Ok, made those changes minus the stacking of the logo text because I think that should adapt to the width of its parent container size instead of a breakpoint if we leave it as live responsive typography.

Alternatively, we can rasterize it as SVG in two different layouts, but that's going to be a pain for localization as new assets will need to be generated for every language.

alanmoo commented 6 years ago

Merging, Rebecca will create a new ticket with remaining QA items.