Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| metricName={metricName} | ||
| timeRange={timeRange} | ||
| labelSelectors={labelSelectors} | ||
| yAxisMin='auto' |
There was a problem hiding this comment.
we need to sync this with backend creating auto charts. @sjaanus do you create auto or starting form zero in the backend code?
|
|
||
| if (!__name__ && !labelParts) { | ||
| return 'Series'; | ||
| return 'Value'; |
There was a problem hiding this comment.
previous name was misleading when hovering over line and seeing "series"
| : undefined; | ||
| const maxTime = end ? fromUnixTime(Number.parseInt(end, 10)) : undefined; | ||
|
|
||
| const placeholder = metricName ? ( |
There was a problem hiding this comment.
untangling double nested ternaries
| noSeriesPlaceholder?: ReactNode; | ||
| isPreview?: boolean; | ||
| showComponents?: ChartComponent[]; | ||
| thresholdCondition?: ThresholdCondition; |
There was a problem hiding this comment.
we don't need threshold and operator anymore, only threshold value to draw the line
| }; | ||
|
|
||
| // This is matching backend logic which looks for a single outlier | ||
| const isThresholdConditionMet = ( |
There was a problem hiding this comment.
we don't change line color based on threshold exceeded anymore
| sx={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| gap: 0.25, | ||
| whiteSpace: 'nowrap', | ||
| textDecoration: 'none', | ||
| fontWeight: 'bold', | ||
| fontSize: 'body2.fontSize', | ||
| color: 'primary.main', | ||
| '&:hover': { | ||
| textDecoration: 'none', | ||
| }, | ||
| }} |
There was a problem hiding this comment.
Use styled component, not such huge sx.
There was a problem hiding this comment.
yeap, had it stashed locally
There was a problem hiding this comment.
While you are touching this file, SafeguardFormBase should have props type defined, not inlined. It already has 7 properties.
| metricOptions.find((m) => m.name === metricName) | ||
| ?.displayName |
There was a problem hiding this comment.
This should be a variable declared before.
sjaanus
left a comment
sjaanus
left a comment
There was a problem hiding this comment.
LG, but with minor code style issues.
2 participants
About the changes
mini chart when configuring safeguard. auto updates on a change to any form field
on hover you get full chart with a link to feature impact metrics
Important files
Discussion points