UI Scroll layout for the graphs #242
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Video looks great! 👏
In terms of implementation, if we want to make a change to layout or styling we now need to make it in 6 places and keep things in parallel - this is super hard and an easy way to introduce mistakes and inconsistencies.
I'd recommend having a crack at pattern like this -
- A single
GraphCardcomponent - It accepts a title prop
- It accepts
children(ie the "content" / graphs - have a look atReact.PropsWithChildren) - It's responsible for layout, next button, scroll, title etc - but not the actual content
app/components/ui/Dashboard.tsx
Outdated
| import { Household } from "@/app/models/Household"; | ||
|
|
||
| interface DashboardProps { | ||
| data: Household; | ||
| } | ||
|
|
||
| const Dashboard: React.FC<DashboardProps> = ({ data }) => { | ||
| const handleNext = () => { | ||
| const container = document.querySelector(".snap-scroll") as HTMLElement; |
There was a problem hiding this comment.
We should be aiming to use ref in React instead of calling native DOM functionality like this.
There was a problem hiding this comment.
Does this version look better? Sorry I wasn't familiar with the children concept , it seems pretty powerful.
If I understand it correctly, it means I can pass other optional React components to the card. For example, I could pass a title AND a graph, or a title AND a section. So the basically the GraphCard component should provide the 'scrollability feature' and render the content that is passed to it, is that correct?
There was a problem hiding this comment.
Perfect! Super quick turnaround 🏅
app/components/ui/Dashboard.tsx
Outdated
| // Dummy identity function to avoid linting errors | ||
| const identity = (data: Household) => { | ||
| return data; | ||
| }; | ||
| identity(data); |
There was a problem hiding this comment.
I don't really follow what this is for sorry!
There was a problem hiding this comment.
Oh sorry, it was a trick to avoid listing errors. We are passing data to the Dashboard component. That makes sense because data will be necessary to generate the plots in the future.
The problem is that right now we are doing nothing with it, so it flags an error of a variable not being used. So I created a function that returns itself for the moment to get around it. I considered passing a plot to one of the Card, but I figured it would be better to be a separate PR
There was a problem hiding this comment.
Ah right! That makes sense - thanks for the explanation 👍
There's an easy option here! We can add // eslint-disable-next-line <RULE_NAME> as a comment to the line above an unused variable we want to keep and it should resolve this issue.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const someUnusedVariable = 123Docs: https://eslint.org/docs/latest/use/configure/rules#disabling-rules
app/components/ui/Dashboard.tsx
Outdated
| import { Household } from "@/app/models/Household"; | ||
|
|
||
| interface DashboardProps { | ||
| data: Household; | ||
| } | ||
|
|
||
| const Dashboard: React.FC<DashboardProps> = ({ data }) => { | ||
| const handleNext = () => { | ||
| const container = document.querySelector(".snap-scroll") as HTMLElement; |
There was a problem hiding this comment.
Perfect! Super quick turnaround 🏅
app/components/ui/Dashboard.tsx
Outdated
| <button | ||
| onClick={handleNext} | ||
| className="px-4 py-2 bg-blue-500 text-white rounded hover:bg-blue-600" | ||
| > | ||
| Next | ||
| </button> |
There was a problem hiding this comment.
Just realised - this currently remains visible on the final card.
A few ways we could handle this - a quick/simple version might be to just to track the index of the pages on handleNext(), and hide for the final one.
There was a problem hiding this comment.
Thanks for the suggestion! Hopefully this will do
There was a problem hiding this comment.
Screen.Recording.2025-01-09.at.09.57.41.mov
What does this PR do?
It sets a series of scrollable layouts for where the graphs are going to live.
The scrolling occurs between
GraphCard1andGraphCard6, which will show the different graphs and text.Screen.Recording.2025-01-08.at.15.41.38.mov