-
Notifications
You must be signed in to change notification settings - Fork 114
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
Update Dashboard UI #1002
Update Dashboard UI #1002
Conversation
ctyrrellnrel
commented
Jul 12, 2023
- Very basic, just replaced normal details view of 'My distance' with chart,
- next steps
- switch 'details' view to a react component
- create button to switch between chart and details view in 'my distance' card
- Figure out why all data is showing up unlabled
- Allow the axis to switch easily
- right now, setting isHorizontal doesn't completely switch data, only thedirection of the bars
- would be nice to flip data axis and bar axis simultaneously
- replaced normal details view of 'My distance' with chart - next steps - create button to switch between chart and details view in 'my distance' card - Figure out why all data is showing up unlabled - Allow the axis to switch easily - right now, setting isHorizontal doesn't completely switch data, only thedirection of the bars - would be nice to flip data axis and bar axis simultaneously
I will be adding images when I get farther in creating the component. |
- one (MetricDetails) is going to be a copy of the original metric details view - the next (MetricsCard) is going to be a container to have both the BarChart component, and the MetricDetails component - Will be a react component, rather than function, and will hold onto the state of whether or not the barchart or metricdetails will be showing - next steps - add a button to switch between the two views
- allows the card to keep track of whether or not it has a BarChart or MetricDetails child component - next step - Add button
- next step - format and style button
- button allows switching between the BarChart and the MetricDetails card - button icon switches depending on the card represented
Current State of PRcurrently working to create react components for all of the cards, that include the new dashboard designs, to see if those work well.
ImagesThis shows the ability to switch between views using the right button in the 'my distance' card the styling is a bit funky right now, but will be solved later on |
Good progress! I'll review the code later |
const { colors } = useTheme(); | ||
const [ numVisibleDatasets, setNumVisibleDatasets ] = useState(1); | ||
|
||
const barChartRef = useRef<Chart>(null); | ||
|
||
const defaultPalette = [ | ||
const defaultPalette = [ |
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.
Try to avoid committing needless changes like this, where you add a space or adjust indentation
I know this was probably on accident - just make sure you check over the diff of what you're committing before you do so
www/js/components/MetricsCard.tsx
Outdated
'#80afad', // teal oklch(72% 0.05 192) | ||
] | ||
return ( | ||
<Card children={{}} style={{width:"90%", alignSelf:"center", height:"280px" }}> |
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.
You don't have to include children
as a prop. I know VSCode probably yelled at you about it being missing.
As long as there are components nested inside you can ignore that warning. It's some issue with VSCode, Typescript, and React.
const { colors } = useTheme(); | ||
const [ numVisibleDatasets, setNumVisibleDatasets ] = useState(1); | ||
|
||
const defaultPalette = [ |
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.
I'm guessing this was copied over from BarChart
. It doesn't need to be here though
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.
If you want to copy the contents of a file as a starting point for a new file, that's ok -- just make sure you remove the bits that aren't relevant.
we don't want unused code lying around
Alright, yeah I was planning on taking it out later, but I guess it's best to simply take it out immediately.
Also I saw that, good to know that it is not required, it seemed like a weird requirement to have. |
- removed children property from card - removed leftover code from copying from BarChart.tsx to MetricsCard and MetricDetails - Removed unecessary import statements
@ctyrrellnrel Some tips while you are working on these TODO items:
|
-- also switched main-metrics html so that only the MetricsCard element is held in the ion-slide element, without any extra divs or headers
Closing this since it has been superceded by #1018 |