Dynamic year filter #155
Dynamic year filter #155
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
This looks great to me!
On the naming of values prop passed to the Filter component, I don't think we're very consistent with that. I think the plural is definitely more readable here!
The only argument I can think of for using the singular would maaaybe be in the case where the Filter is operating like a radio button, so the values value really reflects a single "active" value. But the values object is always going to contain multiple keys and represent the state for multiple options, so even in that case plural might be better 🤔
components/main.js
Outdated
| 2021: true, | ||
| 2022: true, | ||
| 2023: true, | ||
| const currentYear = new Date().getFullYear() |
There was a problem hiding this comment.
Ooh actually, I'm realizing that this will probably be evaluated at build time, so might require triggering a new deployment in order to update around the New Year. That wouldn't be a huge deal, but turning this into a function that gets invoked within Main (similar to what you did in the Footer PR) should ensure we always render the latest!
There was a problem hiding this comment.
ah yep that makes sense! gotta get my mind in the ssr world!
The year list is dynamic now!
I made things more complicated tho -- I changed the variable names to use
yearsplural for what feels like more clarity, but I may not be conceptualizing things correctly cause I see the singular naming on the press page for the format variable...https://github.com/carbonplan/carbonplan.org/blob/7827a44a218dd8ce01caf8de1986edd3d4272baa/pages/press.js#L105-L109
Any reason to lean one way or the other on this?
Not a huge thing obviously, just curious!
cc @katamartin