feat(UI): Another batch of minor improvements #213
Conversation
Unit test coverage
|
| export function CodeContainer({ dataSourceUid, functionDetails }: CodeContainerProps) { | ||
| const { data, actions } = useCodeContainer(dataSourceUid, functionDetails); | ||
|
|
||
| if (data.fetchError && (data.fetchError as HttpClientError)?.response?.status !== 404) { | ||
| displayError(data.fetchError, ['Failed to fetch file information!', (data.fetchError as Error).message]); | ||
| } | ||
|
|
||
| const codeLines: CodeLine[] = data.lines.map((line) => ({ ...line, line: line.line || '???' })); |
There was a problem hiding this comment.
grafakus
changed the title
src/pages/ProfilesExplorerView/components/SceneProfilesExplorer/SceneProfilesExplorer.tsx
Outdated
Show resolved
Hide resolved
| // preserve existing filters only when switching to "Labels" or "Flame graph" | ||
| if (![ExplorationType.LABELS, ExplorationType.FLAME_GRAPH].includes(nextExplorationType as ExplorationType)) { | ||
| // preserve existing filters only when switching to "Labels", "Flame graph" or "Diff flamge graph" | ||
| if ( |
There was a problem hiding this comment.
Definitely a great improvement 🙌
One small nit: I was a bit surprised with following flow:
- I set some labels in Labels view
- I go to Diff View and remove (or change both values to something else)
- I switch back to Labels view (the labels are restored)
If I remove all labels in Diff View and go to labels I kind of expected to have no labels. I'm not sure how it should work in general though 😅 There are multiple cases (one label changed or removed), so it's not easy to make it super clear all the time. An alternative would be to have another input with "common labels" applied to both sides of the view + side filters added to add more granular labels. But that could add more noise to the UI 🤷
Not a blocker, just food for thought. Something we can keep an eye on and double check with UX.
There was a problem hiding this comment.
Thank you for the feedback. It's a tricky one indeed for the cases you mentioned. Let's see with UX.
…r/SceneProfilesExplorer.tsx Co-authored-by: Piotr Jamróz <[email protected]>
✨ Description
Related issue(s):
-📖 Summary of the changes
Code component
Very small refactor to better separate concerns better (follow-up of a previous PR)
Plugin info dropdown
Added a link to our CHANGELOG and a link for reporting an issue:
Diff flame graph view:
Initialize filters when coming from "Labels" or "Flame graph", here's a short video:
Screen.Recording.2024-10-04.at.17.54.42.mov
🧪 How to test?
Manually.