-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: introduce global footer #15
base: main
Are you sure you want to change the base?
feat: introduce global footer #15
Conversation
✅ Deploy Preview for twc-site-nl ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Include a footer on each page which links through to the global TWC page.
a0910e3
to
1fe9eb4
Compare
body > footer { | ||
position: sticky; | ||
top: 100vh; | ||
} |
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.
Based on the approach documented here: https://css-tricks.com/a-clever-sticky-footer-technique/
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.
hmmm. I think I'd rather apply a flex to both the body and the footer. Sticky becomes problematic if the css for the content isn't consistent/allows for it.
// contrast ex: filter: contrast(200%); | ||
// blur ex: filter: blur(2px); | ||
|
||
@mixin filter($filter-type,$filter-amount) { |
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.
Maybe refactor to a global mixins file, but this seems like an optimization to make once we are using it in multiple places.
@thisislawatts curious as to the motivation for this one. It does look nice, however im not sure the utility of this instead of linking back to global on the about page for example. I can see it being useful for bringing attention to larger events etc, but not sure if we need this on all the time. thoughts? |
Good question @b-n, the main motivation was to introduce some balance to the layout. I am pretty open to the content it could contain but the site feels a little top heavy without that element. More generally footers can offer a consistent, easily accessible location for key information and navigation links. In our case we could adopt an extended/secondary footer area to include the primary call to action "Get Involved!" form which is otherwise referenced on almost all our other pages. |
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.
Ohhhhh, I see now. I thought the footer was always visible (e.g. fixed to the viewport), but I just checked the preview out and I see its like a standard content footer etc. Yeah agreed, this feels a lot more balanced etc.
|
||
.footer { | ||
border-top: 1px solid; | ||
padding: 2em 2em; |
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.
too much padding IMHO. Can you try something more proportional?
padding: 1rem 1.5rem;
Maybe that's something we can do with a css cleanup - get some good ratios in.
@thisislawatts Ah sorry, I just realised you were not added to this repo. I've just added you so you can make branches directly from this repo instead of having to work from a fork (I had plans on updating this branch after prettier changes 😅). |
@thisislawatts Hey hey - Just wondering if you're interested in merging this? Would need a rebase off main - Also, am happy to add you as contributor still so you can do direct pull requests if that's easier 😄 |
Include a footer on each page which could in time be expanded to contain a broader range of content but initially it only
links through to the global TWC page.
Attached are screengrabs showing it will appear on mobile and desktop devices.