diff --git a/.betterer.results b/.betterer.results index 1d8794210a1..14165ffed3a 100644 --- a/.betterer.results +++ b/.betterer.results @@ -653,9 +653,7 @@ exports[`better eslint`] = { ], "packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Styles should be written using objects.", "1"], - [0, 0, 0, "Styles should be written using objects.", "2"], - [0, 0, 0, "Styles should be written using objects.", "3"] + [0, 0, 0, "Styles should be written using objects.", "1"] ], "packages/grafana-runtime/src/analytics/types.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] diff --git a/packages/grafana-flamegraph/src/FlameGraph/FlameGraph.tsx b/packages/grafana-flamegraph/src/FlameGraph/FlameGraph.tsx index 31b0b11e58c..777fac6ecd2 100644 --- a/packages/grafana-flamegraph/src/FlameGraph/FlameGraph.tsx +++ b/packages/grafana-flamegraph/src/FlameGraph/FlameGraph.tsx @@ -178,7 +178,6 @@ const getStyles = () => ({ graph: css` label: graph; overflow: auto; - height: 100%; flex-grow: 1; flex-basis: 50%; `, diff --git a/packages/grafana-flamegraph/src/FlameGraph/FlameGraphCanvas.tsx b/packages/grafana-flamegraph/src/FlameGraph/FlameGraphCanvas.tsx index a4d60083c44..724376d96ce 100644 --- a/packages/grafana-flamegraph/src/FlameGraph/FlameGraphCanvas.tsx +++ b/packages/grafana-flamegraph/src/FlameGraph/FlameGraphCanvas.tsx @@ -245,7 +245,6 @@ const getStyles = () => ({ graph: css({ label: 'graph', overflow: 'auto', - height: '100%', flexGrow: 1, flexBasis: '50%', }), diff --git a/packages/grafana-flamegraph/src/FlameGraphContainer.tsx b/packages/grafana-flamegraph/src/FlameGraphContainer.tsx index 05c017789eb..8cb7c89511c 100644 --- a/packages/grafana-flamegraph/src/FlameGraphContainer.tsx +++ b/packages/grafana-flamegraph/src/FlameGraphContainer.tsx @@ -98,7 +98,7 @@ const FlameGraphContainer = ({ return new FlameGraphDataContainer(data, { collapsing: !disableCollapsing }, theme); }, [data, theme, disableCollapsing]); const [colorScheme, setColorScheme] = useColorScheme(dataContainer); - const styles = getStyles(theme, vertical); + const styles = getStyles(theme); // If user resizes window with both as the selected view useEffect(() => { @@ -144,6 +144,65 @@ const FlameGraphContainer = ({ return null; } + const flameGraph = ( + setFocusedItemData(data)} + focusedItemData={focusedItemData} + textAlign={textAlign} + sandwichItem={sandwichItem} + onSandwich={(label: string) => { + resetFocus(); + setSandwichItem(label); + }} + onFocusPillClick={resetFocus} + onSandwichPillClick={resetSandwich} + colorScheme={colorScheme} + showFlameGraphOnly={showFlameGraphOnly} + collapsing={!disableCollapsing} + /> + ); + + const table = ( + + ); + + let body; + if (showFlameGraphOnly || selectedView === SelectedView.FlameGraph) { + body = flameGraph; + } else if (selectedView === SelectedView.TopTable) { + body =
{table}
; + } else if (selectedView === SelectedView.Both) { + if (vertical) { + body = ( +
+
{flameGraph}
+
{table}
+
+ ); + } else { + body = ( +
+
{table}
+
{flameGraph}
+
+ ); + } + } + return ( // We add the theme context to bridge the gap if this is rendered in non grafana environment where the context // isn't already provided. @@ -178,45 +237,7 @@ const FlameGraphContainer = ({ /> )} -
- {!showFlameGraphOnly && selectedView !== SelectedView.FlameGraph && ( - - )} - - {selectedView !== SelectedView.TopTable && ( - setFocusedItemData(data)} - focusedItemData={focusedItemData} - textAlign={textAlign} - sandwichItem={sandwichItem} - onSandwich={(label: string) => { - resetFocus(); - setSandwichItem(label); - }} - onFocusPillClick={resetFocus} - onSandwichPillClick={resetSandwich} - colorScheme={colorScheme} - showFlameGraphOnly={showFlameGraphOnly} - collapsing={!disableCollapsing} - /> - )} -
+
{body}
); @@ -234,12 +255,13 @@ function useColorScheme(dataContainer: FlameGraphDataContainer | undefined) { return [colorScheme, setColorScheme] as const; } -function getStyles(theme: GrafanaTheme2, vertical?: boolean) { +function getStyles(theme: GrafanaTheme2) { return { container: css({ label: 'container', + overflow: 'auto', height: '100%', - display: vertical ? 'block' : 'flex', + display: 'flex', flex: '1 1 0', flexDirection: 'column', minHeight: 0, @@ -247,12 +269,39 @@ function getStyles(theme: GrafanaTheme2, vertical?: boolean) { }), body: css({ label: 'body', - display: 'flex', flexGrow: 1, + }), + + tableContainer: css({ + // This is not ideal for dashboard panel where it creates a double scroll. In a panel it should be 100% but then + // in explore we need a specific height. + height: 800, + }), + + horizontalContainer: css({ + label: 'horizontalContainer', + display: 'flex', minHeight: 0, - height: vertical ? undefined : '100vh', - flexDirection: vertical ? 'column-reverse' : 'row', + flexDirection: 'row', columnGap: theme.spacing(1), + width: '100%', + }), + + horizontalGraphContainer: css({ + flexBasis: '50%', + }), + + horizontalTableContainer: css({ + flexBasis: '50%', + maxHeight: 800, + }), + + verticalGraphContainer: css({ + marginBottom: theme.spacing(1), + }), + + verticalTableContainer: css({ + height: 800, }), }; } diff --git a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx index cef80bc9806..1aeb7d30a56 100644 --- a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx +++ b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx @@ -29,17 +29,15 @@ import { TableData } from '../types'; type Props = { data: FlameGraphDataContainer; onSymbolClick: (symbol: string) => void; - height?: number; search?: string; sandwichItem?: string; onSearch: (str: string) => void; onSandwich: (str?: string) => void; onTableSort?: (sort: string) => void; - vertical?: boolean; }; const FlameGraphTopTableContainer = React.memo( - ({ data, onSymbolClick, height, search, onSearch, sandwichItem, onSandwich, onTableSort, vertical }: Props) => { + ({ data, onSymbolClick, search, onSearch, sandwichItem, onSandwich, onTableSort }: Props) => { const table = useMemo(() => { // Group the data by label, we show only one row per label and sum the values // TODO: should be by filename + funcName + linenumber? @@ -57,21 +55,14 @@ const FlameGraphTopTableContainer = React.memo( return table; }, [data]); - const rowHeight = 35; - // When we use normal layout we size the table to have the same height as the flamegraph to look good side by side. - // In vertical layout we don't need that so this is a bit arbitrary. We want some max limit - // so we don't show potentially thousands of rows at once which can hinder performance (the table is virtualized - // so with some max height it handles it fine) - const tableHeight = vertical ? Math.min(Object.keys(table).length * rowHeight, 800) : 0; - - const styles = useStyles2(getStyles, tableHeight); + const styles = useStyles2(getStyles); const theme = useTheme2(); const [sort, setSort] = useState([{ displayName: 'Self', desc: true }]); return (
- + {({ width, height }) => { if (width < 3 || height < 3) { return null; @@ -319,21 +310,14 @@ function ActionCell(props: ActionCellProps) { ); } -const getStyles = (theme: GrafanaTheme2, height: number) => { +const getStyles = (theme: GrafanaTheme2) => { return { - topTableContainer: css` - label: topTableContainer; - flex-grow: 1; - flex-basis: 50%; - overflow: hidden; - padding: ${theme.spacing(1)}; - background-color: ${theme.colors.background.secondary}; - ${height - ? css` - min-height: ${height}px; - ` - : ''} - `, + topTableContainer: css({ + label: 'topTableContainer', + padding: theme.spacing(1), + backgroundColor: theme.colors.background.secondary, + height: '100%', + }), }; };