feat: add error logs only toggle to logs panel in timepoint explorer #1489
Conversation
markjmeier
requested review from
ckbedwell and
w1kman
and removed request for
a team
Script size changes
Totals
|
w1kman
left a comment
w1kman
left a comment
There was a problem hiding this comment.
The component explodes when "Error logs only" is enabled at the same time as "Raw logs".
explosion.mp4
IF we intend to fix the raw logs switch, I'd suggest moving the state outside of the two components, so that they share the state of the Switch. It somewhat frustrating to toggle the switch again when switching to raw and vice versa.
The switch
- The label for the switch has a
pointer, but clicking it does nothing. - The label text/color does not follow the styles of a form field
switch-pointer.mp4
Suggestion: Use an InlineField (with appropriate flex-box styling) with a Switch as child. It will allow for the label to be clickable as well as styling the label.
No empty state
When there are no error logs, there is "nothing".
Consider adding an empty state that clearly indicates that stuff is hidden and can be displayed
Thoughts
Ideally, it should be possible to filter on any log level.
|
|
||
| export const LogsRaw = <T extends UnknownParsedLokiRecord>({ logs }: { logs: T[] }) => { | ||
| const [width, setWidth] = useState(0); | ||
| const [errorOnly, setErrorOnly] = useState(false); |
There was a problem hiding this comment.
Nit-ish: errorLogsOnly seems like a better fit, based on what it controls.
| <div> | ||
| <Box paddingBottom={1} display="flex" justifyContent="flex-end"> | ||
| <InlineSwitch | ||
| label="Error logs only" |
There was a problem hiding this comment.
Enabling this will throw an error
| mainKey: string; | ||
| }) => { | ||
| const styles = useStyles2(getStyles); | ||
| const [errorOnly, setErrorOnly] = useState(false); |
There was a problem hiding this comment.
Nit-ish: errorLogsOnly seems like a better fit, based on what it controls.
2 participants
What this PR does / why we need it:
Introduces a toggle to only show error logs. For checks with lots of logs, these can be hard to find/surface.
It's worth considering if we should default this to on, if there are any error logs present. We should also consider if theres potentially a better way to filter logs. this is a quick and dirty fix for users with a large number of logs lines in a check
Which issue(s) this PR fixes:
n/a
Fixes #
Special notes for your reviewer: