-
Notifications
You must be signed in to change notification settings - Fork 153
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
Buyers Guide: addressed nav qa issues #1943
Conversation
Design review:
Mobile overlay:
|
…ndation.mozilla.org into issue-1893-bg-nav-qa
…y-nav.scss & some styling fixes
@@ -0,0 +1,56 @@ | |||
<div id="primary-nav-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
following the code hierarchy pattern we have for the main Foundation site nav (network-api/networkapi/templates/pages/primary_nav.html
)
@@ -1,16 +1,13 @@ | |||
.moz-logo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleaned up this file a bit by moving relevant code to primary-nav/primary-nav.scss
Awesome! Desktop nav is missing the new link share icon but that's not a blocker. |
<div class="col"> | ||
<h3 class="h3-heading">Help us keep the Internet healthy</h3> | ||
<p>Mozilla is a nonprofit organization fighting for a healthy interent.</p> | ||
<a class="btn btn-donate m-0 w-100" href="https://donate.mozilla.org">Donate</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need the same utm query args as the footer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pomax I'm not sure - this chunk of code was already in the codebase. I only moved it to this file. Let's file a followup ticket to get clarification then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filed #1959
<div class="narrow-screen-menu-background"> | ||
<div class="narrow-screen-menu-container"> | ||
<div class="social d-flex flex-column"> | ||
<a class="social-button body-large social-button-fb" href="#TODO">Facebook</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we get away with just not setting an href
at all?
@@ -0,0 +1,37 @@ | |||
<?xml version="1.0" encoding="utf-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you'll want to run this through svgo, because this is probably <300byte after svg optimization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrrr+
Closes #1893
Review without whitespace changes:
https://github.com/mozilla/foundation.mozilla.org/pull/1943/files?w=1
Note: share icon functionality will be implemented in a follow-up PR. This PR only covers the UI part of it.