935 trend legend #3434
935 trend legend #3434
Conversation
| logic={trendsLogic} | ||
| props={{ dashboardItemId: null, view: activeView }} | ||
| > | ||
| <TrendLegend /> |
There was a problem hiding this comment.
I think we should only display the legend if there's something to breakdown by - either breakdown or multiple entities graphed.
Otherwise it's a waste of space most times.
There was a problem hiding this comment.
Agreed pending conclusion below (whether we show values or not)
| import { PHCheckbox } from 'lib/components/PHCheckbox' | ||
| import { getChartColors } from 'lib/colors' | ||
|
|
||
| export function TrendLegend(): JSX.Element { |
There was a problem hiding this comment.
Q: Why do we show the values here? It feels like it duplicates the graph, makes the table view unneeded and makes the whole thing too wide to fit on the side.
There was a problem hiding this comment.
I went this route because putting the legend on the right side of the trend graph would add a lot of horizontal density. By putting the legend below, there's a lot of space so I figured we could just fill up the exact numbers for easier comparison.
tagging @timgl @corywatilo for drive by thoughts
There was a problem hiding this comment.
I like the table, especially when breaking down it's nice to get the numbers as well. When I'm using amplitude I found myself looking at the table below a lot to get the precise numbers.
I think we should add something like this too, definitely for dashboards at least.
I'm not super convinced about the
and the spacing. Doesn't look quite right to me. Also i think the table should hit the sides?
(also made a quick commit to make the table small)
|
How do we want to handle overflows? This looks quite bad on linux:
|
|
Overflow would just be scrolling. Hard to get around that because you can filter something for 90 days where you definitely will need to scroll |
There was a problem hiding this comment.
Let's ship this and see the feedback.
Couple of UX suggestions:
- Disable the last checkbox - it'd be weird if user ever got to a "no graph" situtation
- Make the whole cell with the label clickable rather than only the checkbox. Makes it easier to toggle things.
Changes
Please describe.
If this affects the frontend, include screenshots.
Checklist