Feat: impact metrics fronted #10182
Feat: impact metrics fronted #10182
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| const theme = useTheme(); | ||
| const [selectedSeries, setSelectedSeries] = useState<string>(''); | ||
| const [selectedRange, setSelectedRange] = useState< | ||
| 'hour' | 'day' | 'week' | 'month' |
There was a problem hiding this comment.
a lot of type duplication will be fixed when I generate backend types
| ? () => fetcher(formatApiPath(PATH), 'Impact metrics data') | ||
| : () => Promise.resolve([]), | ||
| { | ||
| refreshInterval: 30 * 1_000, |
There was a problem hiding this comment.
pull new data every 30s (opinion)
| overrideOptions={ | ||
| shouldShowPlaceholder | ||
| ? {} | ||
| : { |
There was a problem hiding this comment.
there's a lot of of whitespace on the left hear = code complexity (https://codescene.io/docs/guides/technical/complexity-trends.html#what-s-complexity-anyway)
There was a problem hiding this comment.
can we make the file name more explicit to make searching and prompting LLM easier? e.g. impact-chart-formatters?
| }; | ||
|
|
||
| export const getSeriesLabel = (metric: Record<string, string>): string => { | ||
| const { __name__, ...labels } = metric; |
There was a problem hiding this comment.
not obvious what double underscores are
| @@ -0,0 +1,17 @@ | |||
| import { useTheme } from '@mui/material'; | |||
|
|
|||
| export const useSeriesColor = () => { | |||
There was a problem hiding this comment.
I am assuming it's some deterministic hashing for color assignment. However it's not obvious at first sight. Also is it the first time we're solving this problem in our codebase? I thought that variant colors had a similar case
There was a problem hiding this comment.
Yes, because prior code deals with this by assigning colors based on createdAt, instead of hashing.
| selectedRange: 'hour' | 'day' | 'week' | 'month'; | ||
| onRangeChange: (range: 'hour' | 'day' | 'week' | 'month') => void; | ||
| beginAtZero: boolean; | ||
| onBeginAtZeroChange: (beginAtZero: boolean) => void; |
There was a problem hiding this comment.
this one looks different from a typical component prop. what is it doing?
There was a problem hiding this comment.
Charts have an option for Y axis to be auto-adjusted, or always start at 0
| })); | ||
| }, [metadata]); | ||
|
|
||
| const data = useMemo(() => { |
There was a problem hiding this comment.
can it be extracted so that it's not distracting when reading the component?
github-project-automation
bot
moved this from In Progress
to Approved PRs
in Issues and PRs
Impact Metrics - Frontend draft
frontend/src/component/insights/impact-metrics/ImpactMetrics.tsxImpactMetricsControlsuseImpactMetricsData&useImpactMetricsMetadataFor now it's in "Analytics", but I'll move it somewhere else