From 5e27a6f27630336ada81608640f78a3effae81ed Mon Sep 17 00:00:00 2001 From: Joey Tawadrous <90795735+joey-grafana@users.noreply.github.com> Date: Thu, 20 Oct 2022 14:20:48 +0100 Subject: [PATCH] Flame Graph: Fix for dashboard scrolling (#56555) * Flamegraph dash scrolling * Separate scroll for top table and for flame graph * Custom scroll behavior for explore vs vs dash etc and sticky flame graph header * Final UI tweaks * Update tests --- .../explore/FlameGraphExploreContainer.tsx | 6 ++--- .../components/FlameGraph/FlameGraph.test.tsx | 3 ++- .../components/FlameGraph/FlameGraph.tsx | 14 ++++++++--- .../components/FlameGraphContainer.test.tsx | 4 +-- .../components/FlameGraphContainer.tsx | 25 ++++++++++++++++--- .../components/FlameGraphHeader.test.tsx | 3 +++ .../components/FlameGraphHeader.tsx | 15 ++++++++--- .../TopTable/FlameGraphTopTable.tsx | 5 ++++ .../FlameGraphTopTableContainer.test.tsx | 3 ++- .../TopTable/FlameGraphTopTableContainer.tsx | 9 ++++--- .../app/plugins/panel/flamegraph/module.tsx | 4 +-- 11 files changed, 69 insertions(+), 22 deletions(-) diff --git a/public/app/features/explore/FlameGraphExploreContainer.tsx b/public/app/features/explore/FlameGraphExploreContainer.tsx index 817f9070920..ed66d58b236 100644 --- a/public/app/features/explore/FlameGraphExploreContainer.tsx +++ b/public/app/features/explore/FlameGraphExploreContainer.tsx @@ -1,7 +1,7 @@ import { css } from '@emotion/css'; import React from 'react'; -import { DataFrame, GrafanaTheme2 } from '@grafana/data'; +import { DataFrame, GrafanaTheme2, CoreApp } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import FlameGraphContainer from '../../plugins/panel/flamegraph/components/FlameGraphContainer'; @@ -15,7 +15,7 @@ export const FlameGraphExploreContainer = (props: Props) => { return (
- +
); }; @@ -24,7 +24,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ container: css` background: ${theme.colors.background.primary}; display: flow-root; - padding: ${theme.spacing(1)}; + padding: 0 ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(1)}; border: 1px solid ${theme.components.panel.borderColor}; border-radius: ${theme.shape.borderRadius(1)}; `, diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx index a4eb0152507..ff12c0510f9 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx @@ -2,7 +2,7 @@ import { screen } from '@testing-library/dom'; import { render } from '@testing-library/react'; import React, { useState } from 'react'; -import { DataFrameView, MutableDataFrame } from '@grafana/data'; +import { CoreApp, DataFrameView, MutableDataFrame } from '@grafana/data'; import { SelectedView } from '../types'; @@ -33,6 +33,7 @@ describe('FlameGraph', () => { return ( { - const styles = getStyles(selectedView); + const styles = getStyles(selectedView, app, flameGraphHeight); const totalTicks = data.fields[1].values.get(0); const valueField = data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); @@ -173,11 +177,15 @@ const FlameGraph = ({ ); }; -const getStyles = (selectedView: SelectedView) => ({ +const getStyles = (selectedView: SelectedView, app: CoreApp, flameGraphHeight: number | undefined) => ({ graph: css` cursor: pointer; float: left; + overflow: scroll; width: ${selectedView === SelectedView.FlameGraph ? '100%' : '50%'}; + ${app !== CoreApp.Explore + ? `height: calc(${flameGraphHeight}px - 44px)` + : ''}; // 44px to adjust for header pushing content down `, }); diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx index fba4d8f5fe6..c8a1f53eedf 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx @@ -2,7 +2,7 @@ import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import React from 'react'; -import { MutableDataFrame } from '@grafana/data'; +import { CoreApp, MutableDataFrame } from '@grafana/data'; import { MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH } from '../constants'; @@ -29,7 +29,7 @@ describe('FlameGraphContainer', () => { }, }; - return ; + return ; }; it('should render without error', async () => { diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx index 5faebaa36db..4ef6959a339 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx @@ -1,9 +1,11 @@ +import { css } from '@emotion/css'; import React, { useEffect, useMemo, useState } from 'react'; import { useMeasure } from 'react-use'; -import { DataFrame, DataFrameView } from '@grafana/data'; +import { DataFrame, DataFrameView, CoreApp } from '@grafana/data'; +import { useStyles2 } from '@grafana/ui'; -import { MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH } from '../constants'; +import { MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH, PIXELS_PER_LEVEL } from '../constants'; import FlameGraph from './FlameGraph/FlameGraph'; import { Item, nestedSetToLevels } from './FlameGraph/dataTransform'; @@ -13,6 +15,11 @@ import { SelectedView } from './types'; type Props = { data: DataFrame; + app: CoreApp; + // Height for flame graph when not used in explore. + // This needs to be different to explore flame graph height as we + // use panels with user adjustable heights in dashboards etc. + flameGraphHeight?: number; }; const FlameGraphContainer = (props: Props) => { @@ -34,6 +41,8 @@ const FlameGraphContainer = (props: Props) => { return nestedSetToLevels(dataView); }, [props.data]); + const styles = useStyles2(() => getStyles(props.app, PIXELS_PER_LEVEL * levels.length)); + // If user resizes window with both as the selected view useEffect(() => { if ( @@ -46,8 +55,9 @@ const FlameGraphContainer = (props: Props) => { }, [selectedView, setSelectedView, containerWidth]); return ( -
+
{ {selectedView !== SelectedView.FlameGraph && ( { {selectedView !== SelectedView.TopTable && ( { ); }; +const getStyles = (app: CoreApp, height: number) => ({ + container: css` + height: ${app === CoreApp.Explore ? height + 'px' : '100%'}; + `, +}); + export default FlameGraphContainer; diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.test.tsx index 73f384f8ad6..b11786c7abe 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.test.tsx @@ -3,6 +3,8 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React, { useState } from 'react'; +import { CoreApp } from '@grafana/data'; + import FlameGraphHeader from './FlameGraphHeader'; import { SelectedView } from './types'; @@ -13,6 +15,7 @@ describe('FlameGraphHeader', () => { return ( void; setRangeMin: (range: number) => void; @@ -19,6 +21,7 @@ type Props = { }; const FlameGraphHeader = ({ + app, search, setTopLevelIndex, setRangeMin, @@ -28,7 +31,7 @@ const FlameGraphHeader = ({ setSelectedView, containerWidth, }: Props) => { - const styles = useStyles(getStyles); + const styles = useStyles2((theme) => getStyles(theme, app)); let viewOptions: Array<{ value: string; label: string; description: string }> = [ { value: SelectedView.TopTable, label: 'Top Table', description: 'Only show top table' }, @@ -83,11 +86,15 @@ const FlameGraphHeader = ({ ); }; -const getStyles = () => ({ +const getStyles = (theme: GrafanaTheme2, app: CoreApp) => ({ header: css` display: flow-root; - padding: 0 0 20px 0; width: 100%; + background: ${theme.colors.background.primary}; + top: 0; + height: 50px; + z-index: ${theme.zIndex.navbarFixed}; + ${app === CoreApp.Explore ? 'position: sticky; margin-bottom: 8px; padding-top: 9px' : ''}; `, inputContainer: css` float: left; diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx index ea43f115ae2..8f75fa32eb9 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx @@ -203,6 +203,11 @@ const getStyles = (theme: GrafanaTheme2) => ({ & > :nth-child(3) { text-align: right; } + + // needed to keep header row height fixed so header row does not resize with browser + & > :nth-child(3) { + position: relative !important; + } `, headerCell: css` background-color: ${theme.colors.background.secondary}; diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx index b76f2d2fd3f..b70a93a0895 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx @@ -1,7 +1,7 @@ import { render, screen } from '@testing-library/react'; import React, { useState } from 'react'; -import { DataFrameView, MutableDataFrame } from '@grafana/data'; +import { CoreApp, DataFrameView, MutableDataFrame } from '@grafana/data'; import { Item, nestedSetToLevels } from '../FlameGraph/dataTransform'; import { data } from '../FlameGraph/testData/dataNestedSet'; @@ -21,6 +21,7 @@ describe('FlameGraphTopTableContainer', () => { return ( { - const styles = useStyles2(() => getStyles(selectedView)); + const styles = useStyles2(() => getStyles(selectedView, app)); const [topTable, setTopTable] = useState(); const valueField = data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); @@ -122,7 +124,7 @@ const FlameGraphTopTableContainer = ({ ); }; -const getStyles = (selectedView: SelectedView) => { +const getStyles = (selectedView: SelectedView, app: CoreApp) => { const marginRight = '20px'; return { @@ -131,6 +133,7 @@ const getStyles = (selectedView: SelectedView) => { float: left; margin-right: ${marginRight}; width: ${selectedView === SelectedView.TopTable ? '100%' : `calc(50% - ${marginRight})`}; + ${app !== CoreApp.Explore ? 'height: calc(100% - 44px)' : ''}; // 44px to adjust for header pushing content down `, }; }; diff --git a/public/app/plugins/panel/flamegraph/module.tsx b/public/app/plugins/panel/flamegraph/module.tsx index 5d64acc2f53..b8f883e07fb 100644 --- a/public/app/plugins/panel/flamegraph/module.tsx +++ b/public/app/plugins/panel/flamegraph/module.tsx @@ -1,11 +1,11 @@ import React from 'react'; -import { PanelPlugin, PanelProps } from '@grafana/data'; +import { CoreApp, PanelPlugin, PanelProps } from '@grafana/data'; import FlameGraphContainer from './components/FlameGraphContainer'; export const FlameGraphPanel: React.FunctionComponent = (props) => { - return ; + return ; }; export const plugin = new PanelPlugin(FlameGraphPanel);