-
Notifications
You must be signed in to change notification settings - Fork 78
Add light gray background to footer #421
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
Conversation
3517eca to
f6328ed
Compare
|
LGTM! |
|
digging the color background change.. good to separate it from the rest of the page. The only time it may look awkward is on a short page (so footer would take up majority of real estate). not a big fan of the single column format in footer though, thoughts? |
|
@olegko not 100% sold on the single column footer, but keep in mind that layout is for phones. Similar to heroku.com, codeschool.com ... |
|
@olegko I was thinking that as well. It may work as two columns, left aligned. Social links could be icon only across the bottom |
|
are we planning to add links to the "help" and "learn" sections? Otherwise, subtitles seem unnecessary |
|
@ivy-g good point, maybe we should make the subtitles themselves be links to the email and blog instead of having redundant links? |
|
The single column is how a lot of people go about it on mobile. But we should explore double column options and even a simplified footer. I like the idea of icons only for social. |
|
if we took out the subtitles and simplified the social section, it might look cleaner as a single column. |
|
@appleJax i agree, making help an email link and learn link to the blog might be a lot nicer |
|
Just want to put in that we should create an issue for general footer formatting improvements for both web and mobile views. I think that this is a good discussion and should be retained for further improvements and not removed once this small change gets merged |
|
Yeah, I just realized most of this discussion actually has nothing to do with the gray background, which is the only change this PR makes, lol. All great stuff though, maybe we should move this discussion to PR #418 |
|
@appleJax agreed |
|
So yay or nay on the gray background? |
|
yay |
|
@npendery @olegko @rileytaylor @WenInCode @joshsmith ? yay: 2 |
|
I like it |
|
Ok, I think we have a majority vote to add the gray background. Permission to merge? |
|
|
What's in this PR?
Added a light gray background to the site footer. It's a small change, but it really differentiates the footer, especially on mobile.
Here it is on mobile (note: this screenshot is from a different branch that has a responsive layout):