Add sitemap to footer #868
Conversation
There was a problem hiding this comment.
Nice one @victorlin! I haven't thought too hard about the entries in each list but as a v1 it looks great.
| @@ -0,0 +1,183 @@ | |||
| import React from "react"; | |||
| import styled from "styled-components"; | |||
There was a problem hiding this comment.
FWIW, I'd like us to move away from styled components over time. This isn't a request to stop using it in this PR - we use it everywhere at the moment, so staying consistent is great - but just a heads up as to my thinking of where we'll take this over time.
There was a problem hiding this comment.
Could you write up an issue for this? I remember you also brought it up somewhere else with more reasoning but I can't seem to find it.
There was a problem hiding this comment.
It's really part of that larger discussion issue that I can't find at present -- what frontend do we want? That discussion involves styles, how data is passed between client & server etc.
There was a problem hiding this comment.
Is it this?
We should have a site-wide stylesheet which is enough for most pages to work without custom styling.
|
This will be a nice improvement! Aesthetically, one thing I'd point out (in addition to what James did) is that the alignment/justification of the sitemap as a whole and the columns within it feel a bit off to me. Our central layout element/block is a centered column, but we have a slew of widths for that column. Within the column, text and inline elements are either centered or left-justified.
I think the sitemap would "fit in" better with the rest of the page blocks with a reduced overall width (e.g. what the team block uses) and/or with the inner columns/blocks of the sitemap (Resources, Tools, etc) themselves centered (but keeping the text elements within them still left-justified). Something like the below.
|
victorlin
force-pushed
the
victorlin/footer-sitemap
branch
from
b4727b1
to
ce165a8
Compare
With the new sitemap at the top of the footer, it makes more sense to render this on not only GenericPage but also pages that use Splash.
victorlin
force-pushed
the
victorlin/footer-sitemap
branch
from
ce165a8
to
69c3b5d
Compare
There was a problem hiding this comment.
Thanks @victorlin! (A rebase onto master might be nice before merge, but not required)
|
Thanks for reviewing! There's no merge conflicts and #869 is based on this, so I'll just merge as-is to avoid extra rebase churn on that PR. |
preview on contact page
Description of proposed changes
Adds links to the footer.
Related issue(s)
Checklist
Check if changes affect the resource index JSON revision