Roadmap page #15310
Roadmap page #15310
Conversation
corwintines
requested review from
wackerow,
pettinarip and
minimalsm
as code owners
github-actions
bot
added
content 🖋️
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
github-actions
bot
added
the
dependencies 📦
There was a problem hiding this comment.
nice job @corwintines! looks great. Left a few observations but nothing blocking from my side.
src/styles/semantic-tokens.css
Outdated
| --roadmap-card-gradient: linear-gradient(123deg, rgba(255, 255, 255, 0.20) 58.99%, rgba(174, 110, 203, 0.13) 104.04%); | ||
| --roadmap-upgrade-card-gradient: linear-gradient(95deg, rgba(211, 145, 242, 0.12) 0%, rgba(159, 43, 212, 0.12) 102.78%); | ||
| --roadmap-upgrade-card-gradient-hover: linear-gradient(95deg, rgba(211, 145, 242, 0.2) 0%, rgba(159, 43, 212, 0.2) 102.78%); |
There was a problem hiding this comment.
Would be nice to call these with a more abstract name to reuse it in future cards. Maybe just card-gradient and card-secondary-gradient.
src/components/ui/buttons/Button.tsx
Outdated
| @@ -44,6 +44,7 @@ const buttonVariants = cva( | |||
| lg: "text-lg py-3 px-8 [&>svg]:text-2xl rounded-lg focus-visible:rounded-lg", | |||
| md: "min-h-10.5 px-4 py-2 [&>svg]:text-2xl", | |||
| sm: "text-xs min-h-[31px] py-1.5 px-2 [&>svg]:text-md", | |||
| xs: "text-xs py-1 px-1 [&>svg]:text-md", | |||
There was a problem hiding this comment.
wouldn't add this since it is not part of the DS and would invite ppl to use it while its not what we want.
It seems that we are adding it for a very unique case. If that is correct, I would add these styles inline to that particular button.
src/data/roadmap/releases.tsx
Outdated
| image: DevelopersHubHeroImage, | ||
| releaseName: "Paris (The Merge)", | ||
| releaseDate: "2022-09-15", | ||
| content: [ |
There was a problem hiding this comment.
why is content an array? seems like we are always rendering one node
There was a problem hiding this comment.
Artifact of old idea for how I was going to do this, will refactor.
src/data/roadmap/releases.tsx
Outdated
| releaseName: "Paris (The Merge)", | ||
| releaseDate: "2022-09-15", | ||
| content: [ | ||
| <div key="1"> |
There was a problem hiding this comment.
I guess key is useless here, no?
|
|
||
| const [api1, setApi1] = useState<CarouselApi>() | ||
| const [api2, setApi2] = useState<CarouselApi>() | ||
| const [currentIndex, setCurrentIndex] = useState(findLatestReleaseIndex()) |
There was a problem hiding this comment.
if the intention here is to have an initial value, we should call it with a function (initialized function). Otherwise, findLatestReleaseIndex would be called in every render.
| const [currentIndex, setCurrentIndex] = useState(findLatestReleaseIndex()) | |
| const [currentIndex, setCurrentIndex] = useState(() => findLatestReleaseIndex()) |
|
|
||
| import { releasesData } from "@/data/roadmap/releases" | ||
|
|
||
| const formatReleaseDate = (date: string) => { |
There was a problem hiding this comment.
can we place it in lib/utils/date as formatDate?
| : release.releaseDate < | ||
| new Date().toISOString().split("T")[0] |
There was a problem hiding this comment.
we could reuse some of the vars already defined and have a safer comparison with numbers
| : release.releaseDate < | |
| new Date().toISOString().split("T")[0] | |
| : releaseDate.getTime() < todayDate.getTime() |
update content
Description