From 433657bc5ee52f1a0ddbddca914c08b240d4b2e8 Mon Sep 17 00:00:00 2001 From: k-fish Date: Fri, 22 Oct 2021 14:03:00 -0400 Subject: [PATCH 1/4] feat(perf): Fill out frontend-other tab This fills out the frontend-other tab as well as fixing a couple issues with local storage between tabs (using a provider to add additional context about the current view to the key, the name isn't great though). --- .../contexts/currentPerformanceView.tsx | 18 ++++++ .../landing/views/allTransactionsView.tsx | 54 ++++++++++-------- .../landing/views/frontendOtherView.tsx | 39 ++++++++++--- .../landing/views/frontendPageloadView.tsx | 56 ++++++++++--------- .../widgets/components/widgetContainer.tsx | 42 +++++++++++--- .../landing/widgets/widgetDefinitions.tsx | 8 +++ .../views/performance/landing/index.spec.tsx | 10 ++-- .../landing/widgets/widgetContainer.spec.jsx | 30 ++++++---- 8 files changed, 175 insertions(+), 82 deletions(-) create mode 100644 static/app/utils/performance/contexts/currentPerformanceView.tsx diff --git a/static/app/utils/performance/contexts/currentPerformanceView.tsx b/static/app/utils/performance/contexts/currentPerformanceView.tsx new file mode 100644 index 00000000000000..3786468c164f54 --- /dev/null +++ b/static/app/utils/performance/contexts/currentPerformanceView.tsx @@ -0,0 +1,18 @@ +import {PROJECT_PERFORMANCE_TYPE} from 'app/views/performance/utils'; + +import {createDefinedContext} from './utils'; + +type useCurrentPerformanceView = { + performanceType: PROJECT_PERFORMANCE_TYPE; +}; + +const [CurrentPerformanceViewProvider, _useCurrentPerformanceType] = + createDefinedContext({ + name: 'CurrentPerformanceViewContext', + }); + +export {CurrentPerformanceViewProvider}; + +export function useCurrentPerformanceType(): PROJECT_PERFORMANCE_TYPE { + return _useCurrentPerformanceType().performanceType; +} diff --git a/static/app/views/performance/landing/views/allTransactionsView.tsx b/static/app/views/performance/landing/views/allTransactionsView.tsx index 89c0bc1fd54daa..32b56536cca29a 100644 --- a/static/app/views/performance/landing/views/allTransactionsView.tsx +++ b/static/app/views/performance/landing/views/allTransactionsView.tsx @@ -1,6 +1,8 @@ +import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; import Table from '../../table'; +import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; import {DoubleChartRow, TripleChartRow} from '../widgets/components/widgetChartRow'; import {PerformanceWidgetSetting} from '../widgets/widgetDefinitions'; @@ -8,29 +10,33 @@ import {BasePerformanceViewProps} from './types'; export function AllTransactionsView(props: BasePerformanceViewProps) { return ( -
- - - - + +
+ + +
+ + ); } diff --git a/static/app/views/performance/landing/views/frontendOtherView.tsx b/static/app/views/performance/landing/views/frontendOtherView.tsx index dbc14c72e360bf..d65ba28890615b 100644 --- a/static/app/views/performance/landing/views/frontendOtherView.tsx +++ b/static/app/views/performance/landing/views/frontendOtherView.tsx @@ -1,18 +1,43 @@ +import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; import Table from '../../table'; +import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; import {FRONTEND_OTHER_COLUMN_TITLES} from '../data'; +import {DoubleChartRow, TripleChartRow} from '../widgets/components/widgetChartRow'; +import {PerformanceWidgetSetting} from '../widgets/widgetDefinitions'; import {BasePerformanceViewProps} from './types'; export function FrontendOtherView(props: BasePerformanceViewProps) { return ( -
-
- + +
+ + +
+ + ); } diff --git a/static/app/views/performance/landing/views/frontendPageloadView.tsx b/static/app/views/performance/landing/views/frontendPageloadView.tsx index 7a5bbc66a03f09..bdcd2467b5d314 100644 --- a/static/app/views/performance/landing/views/frontendPageloadView.tsx +++ b/static/app/views/performance/landing/views/frontendPageloadView.tsx @@ -1,6 +1,8 @@ +import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; import Table from '../../table'; +import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; import {FRONTEND_PAGELOAD_COLUMN_TITLES} from '../data'; import {DoubleChartRow, TripleChartRow} from '../widgets/components/widgetChartRow'; import {PerformanceWidgetSetting} from '../widgets/widgetDefinitions'; @@ -9,30 +11,34 @@ import {BasePerformanceViewProps} from './types'; export function FrontendPageloadView(props: BasePerformanceViewProps) { return ( -
- - -
- + +
+ + +
+ + ); } diff --git a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx index 3a42b723291c2f..5a734f85bb13aa 100644 --- a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx +++ b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx @@ -4,9 +4,11 @@ import styled from '@emotion/styled'; import MenuItem from 'app/components/menuItem'; import {Organization} from 'app/types'; import localStorage from 'app/utils/localStorage'; +import {useCurrentPerformanceType} from 'app/utils/performance/contexts/currentPerformanceView'; import {useOrganization} from 'app/utils/useOrganization'; import withOrganization from 'app/utils/withOrganization'; import ContextMenu from 'app/views/dashboardsV2/contextMenu'; +import {PROJECT_PERFORMANCE_TYPE} from 'app/views/performance/utils'; import {GenericPerformanceWidgetDataType} from '../types'; import {PerformanceWidgetSetting, WIDGET_DEFINITIONS} from '../widgetDefinitions'; @@ -29,20 +31,29 @@ type Props = { } & ChartRowProps; // Use local storage for chart settings for now. -const getContainerLocalStorageKey = (index: number, height: number) => - `landing-chart-container#${height}#${index}`; +const getContainerLocalStorageObjectKey = 'landing-chart-container'; +const getContainerKey = ( + index: number, + performanceType: PROJECT_PERFORMANCE_TYPE, + height: number +) => `landing-chart-container#${performanceType}#${height}#${index}`; const getChartSetting = ( index: number, height: number, + performanceType: PROJECT_PERFORMANCE_TYPE, defaultType: PerformanceWidgetSetting, forceDefaultChartSetting?: boolean // Used for testing. ): PerformanceWidgetSetting => { if (forceDefaultChartSetting) { return defaultType; } - const key = getContainerLocalStorageKey(index, height); - const value = localStorage.getItem(key); + const key = getContainerKey(index, performanceType, height); + const localObject = JSON.parse( + localStorage.getItem(getContainerLocalStorageObjectKey) || '{}' + ); + const value = localObject?.[key]; + if ( value && Object.values(PerformanceWidgetSetting).includes(value as PerformanceWidgetSetting) @@ -55,25 +66,38 @@ const getChartSetting = ( const _setChartSetting = ( index: number, height: number, + performanceType: PROJECT_PERFORMANCE_TYPE, setting: PerformanceWidgetSetting ) => { - const key = getContainerLocalStorageKey(index, height); - localStorage.setItem(key, setting); + const key = getContainerKey(index, performanceType, height); + const localObject = JSON.parse( + localStorage.getItem(getContainerLocalStorageObjectKey) || '{}' + ); + localObject[key] = setting; + + localStorage.setItem(getContainerLocalStorageObjectKey, JSON.stringify(localObject)); }; const _WidgetContainer = (props: Props) => { - const {organization, index, chartHeight, ...rest} = props; - const _chartSetting = getChartSetting( + const {organization, index, chartHeight, allowedCharts, ...rest} = props; + const performanceType = useCurrentPerformanceType(); + let _chartSetting = getChartSetting( index, chartHeight, + performanceType, rest.defaultChartSetting, rest.forceDefaultChartSetting ); + + if (!allowedCharts.includes(_chartSetting)) { + _chartSetting = rest.defaultChartSetting; + } + const [chartSetting, setChartSettingState] = useState(_chartSetting); const setChartSetting = (setting: PerformanceWidgetSetting) => { if (!props.forceDefaultChartSetting) { - _setChartSetting(index, chartHeight, setting); + _setChartSetting(index, chartHeight, performanceType, setting); } setChartSettingState(setting); }; diff --git a/static/app/views/performance/landing/widgets/widgetDefinitions.tsx b/static/app/views/performance/landing/widgets/widgetDefinitions.tsx index c181effe5365f0..19a5ea8e0c071e 100644 --- a/static/app/views/performance/landing/widgets/widgetDefinitions.tsx +++ b/static/app/views/performance/landing/widgets/widgetDefinitions.tsx @@ -17,6 +17,7 @@ export interface BaseChartSetting { } export enum PerformanceWidgetSetting { + DURATION_HISTOGRAM = 'duration_histogram', LCP_HISTOGRAM = 'lcp_histogram', FCP_HISTOGRAM = 'fcp_histogram', FID_HISTOGRAM = 'fid_histogram', @@ -43,6 +44,13 @@ export const WIDGET_DEFINITIONS: ({ }: { organization: Organization; }) => ({ + [PerformanceWidgetSetting.DURATION_HISTOGRAM]: { + title: t('Duration Distribution'), + titleTooltip: getTermHelp(organization, PERFORMANCE_TERM.DURATION_DISTRIBUTION), + fields: ['transaction.duration'], + dataType: GenericPerformanceWidgetDataType.histogram, + chartColor: WIDGET_PALETTE[5], + }, [PerformanceWidgetSetting.LCP_HISTOGRAM]: { title: t('LCP Distribution'), titleTooltip: getTermHelp(organization, PERFORMANCE_TERM.DURATION_DISTRIBUTION), diff --git a/tests/js/spec/views/performance/landing/index.spec.tsx b/tests/js/spec/views/performance/landing/index.spec.tsx index c6ed36484f4406..c027103538ca5d 100644 --- a/tests/js/spec/views/performance/landing/index.spec.tsx +++ b/tests/js/spec/views/performance/landing/index.spec.tsx @@ -108,11 +108,11 @@ describe('Performance > Landing > Index', function () { const titles = wrapper.find('div[data-test-id="performance-widget-title"]'); expect(titles).toHaveLength(5); - expect(titles.at(0).text()).toEqual('Transactions Per Minute'); - expect(titles.at(1).text()).toEqual('Most Related Errors'); - expect(titles.at(2).text()).toEqual('p75 LCP'); - expect(titles.at(3).text()).toEqual('LCP Distribution'); - expect(titles.at(4).text()).toEqual('FCP Distribution'); + expect(titles.at(0).text()).toEqual('p75 LCP'); + expect(titles.at(1).text()).toEqual('LCP Distribution'); + expect(titles.at(2).text()).toEqual('FCP Distribution'); + expect(titles.at(3).text()).toEqual('Most Related Errors'); + expect(titles.at(4).text()).toEqual('Most Related Issues'); }); it('renders frontend other view', async function () { diff --git a/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx b/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx index b69bf18e0bfdf6..4afe8100038d96 100644 --- a/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx +++ b/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx @@ -1,24 +1,30 @@ import {mountWithTheme} from 'sentry-test/enzyme'; import {initializeData} from 'sentry-test/performance/initializePerformanceData'; +import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {OrganizationContext} from 'app/views/organizationContext'; import WidgetContainer from 'app/views/performance/landing/widgets/components/widgetContainer'; import {PerformanceWidgetSetting} from 'app/views/performance/landing/widgets/widgetDefinitions'; +import {PROJECT_PERFORMANCE_TYPE} from 'app/views/performance/utils'; const WrappedComponent = ({data, ...rest}) => { return ( - - - + + + + + ); }; From a078fecdbac1648f30af9c33fd96a495a9af2577 Mon Sep 17 00:00:00 2001 From: k-fish Date: Tue, 26 Oct 2021 11:54:20 -0400 Subject: [PATCH 2/4] Change name of performance context --- ...tPerformanceView.tsx => performanceDisplayContext.tsx} | 8 ++++---- .../performance/landing/views/allTransactionsView.tsx | 8 +++----- .../views/performance/landing/views/frontendOtherView.tsx | 6 +++--- .../performance/landing/views/frontendPageloadView.tsx | 6 +++--- .../landing/widgets/components/widgetContainer.tsx | 4 ++-- 5 files changed, 15 insertions(+), 17 deletions(-) rename static/app/utils/performance/contexts/{currentPerformanceView.tsx => performanceDisplayContext.tsx} (57%) diff --git a/static/app/utils/performance/contexts/currentPerformanceView.tsx b/static/app/utils/performance/contexts/performanceDisplayContext.tsx similarity index 57% rename from static/app/utils/performance/contexts/currentPerformanceView.tsx rename to static/app/utils/performance/contexts/performanceDisplayContext.tsx index 3786468c164f54..75159ca3143665 100644 --- a/static/app/utils/performance/contexts/currentPerformanceView.tsx +++ b/static/app/utils/performance/contexts/performanceDisplayContext.tsx @@ -6,13 +6,13 @@ type useCurrentPerformanceView = { performanceType: PROJECT_PERFORMANCE_TYPE; }; -const [CurrentPerformanceViewProvider, _useCurrentPerformanceType] = +const [PerformanceDisplayProvider, _usePerformanceDisplayType] = createDefinedContext({ name: 'CurrentPerformanceViewContext', }); -export {CurrentPerformanceViewProvider}; +export {PerformanceDisplayProvider}; -export function useCurrentPerformanceType(): PROJECT_PERFORMANCE_TYPE { - return _useCurrentPerformanceType().performanceType; +export function usePerformanceDisplayType(): PROJECT_PERFORMANCE_TYPE { + return _usePerformanceDisplayType().performanceType; } diff --git a/static/app/views/performance/landing/views/allTransactionsView.tsx b/static/app/views/performance/landing/views/allTransactionsView.tsx index 32b56536cca29a..635ac7c8213d53 100644 --- a/static/app/views/performance/landing/views/allTransactionsView.tsx +++ b/static/app/views/performance/landing/views/allTransactionsView.tsx @@ -1,5 +1,5 @@ -import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; +import {PerformanceDisplayProvider} from 'app/utils/performance/contexts/performanceDisplayContext'; import Table from '../../table'; import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; @@ -10,9 +10,7 @@ import {BasePerformanceViewProps} from './types'; export function AllTransactionsView(props: BasePerformanceViewProps) { return ( - +
- + ); } diff --git a/static/app/views/performance/landing/views/frontendOtherView.tsx b/static/app/views/performance/landing/views/frontendOtherView.tsx index d65ba28890615b..d6a23a18205f8c 100644 --- a/static/app/views/performance/landing/views/frontendOtherView.tsx +++ b/static/app/views/performance/landing/views/frontendOtherView.tsx @@ -1,5 +1,5 @@ -import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; +import {PerformanceDisplayProvider} from 'app/utils/performance/contexts/performanceDisplayContext'; import Table from '../../table'; import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; @@ -11,7 +11,7 @@ import {BasePerformanceViewProps} from './types'; export function FrontendOtherView(props: BasePerformanceViewProps) { return ( -
@@ -38,6 +38,6 @@ export function FrontendOtherView(props: BasePerformanceViewProps) { setError={usePageError().setPageError} />
- + ); } diff --git a/static/app/views/performance/landing/views/frontendPageloadView.tsx b/static/app/views/performance/landing/views/frontendPageloadView.tsx index bdcd2467b5d314..de0954197606ef 100644 --- a/static/app/views/performance/landing/views/frontendPageloadView.tsx +++ b/static/app/views/performance/landing/views/frontendPageloadView.tsx @@ -1,5 +1,5 @@ -import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; import {usePageError} from 'app/utils/performance/contexts/pageError'; +import {PerformanceDisplayProvider} from 'app/utils/performance/contexts/performanceDisplayContext'; import Table from '../../table'; import {PROJECT_PERFORMANCE_TYPE} from '../../utils'; @@ -11,7 +11,7 @@ import {BasePerformanceViewProps} from './types'; export function FrontendPageloadView(props: BasePerformanceViewProps) { return ( -
@@ -39,6 +39,6 @@ export function FrontendPageloadView(props: BasePerformanceViewProps) { setError={usePageError().setPageError} />
- + ); } diff --git a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx index 5a734f85bb13aa..19e299621a1134 100644 --- a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx +++ b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx @@ -4,7 +4,7 @@ import styled from '@emotion/styled'; import MenuItem from 'app/components/menuItem'; import {Organization} from 'app/types'; import localStorage from 'app/utils/localStorage'; -import {useCurrentPerformanceType} from 'app/utils/performance/contexts/currentPerformanceView'; +import {usePerformanceDisplayType} from 'app/utils/performance/contexts/performanceDisplayContext'; import {useOrganization} from 'app/utils/useOrganization'; import withOrganization from 'app/utils/withOrganization'; import ContextMenu from 'app/views/dashboardsV2/contextMenu'; @@ -80,7 +80,7 @@ const _setChartSetting = ( const _WidgetContainer = (props: Props) => { const {organization, index, chartHeight, allowedCharts, ...rest} = props; - const performanceType = useCurrentPerformanceType(); + const performanceType = usePerformanceDisplayType(); let _chartSetting = getChartSetting( index, chartHeight, From 2355f8ce7a0b0895e201d2e76a4ba60623a9a210 Mon Sep 17 00:00:00 2001 From: k-fish Date: Tue, 26 Oct 2021 11:58:09 -0400 Subject: [PATCH 3/4] Extract the object fetching to local storage into its own functions. --- .../widgets/components/widgetContainer.tsx | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx index 19e299621a1134..edd080c8242b2f 100644 --- a/static/app/views/performance/landing/widgets/components/widgetContainer.tsx +++ b/static/app/views/performance/landing/widgets/components/widgetContainer.tsx @@ -38,6 +38,17 @@ const getContainerKey = ( height: number ) => `landing-chart-container#${performanceType}#${height}#${index}`; +function getWidgetStorageObject() { + const localObject = JSON.parse( + localStorage.getItem(getContainerLocalStorageObjectKey) || '{}' + ); + return localObject; +} + +function setWidgetStorageObject(localObject: Record) { + localStorage.setItem(getContainerLocalStorageObjectKey, JSON.stringify(localObject)); +} + const getChartSetting = ( index: number, height: number, @@ -49,9 +60,7 @@ const getChartSetting = ( return defaultType; } const key = getContainerKey(index, performanceType, height); - const localObject = JSON.parse( - localStorage.getItem(getContainerLocalStorageObjectKey) || '{}' - ); + const localObject = getWidgetStorageObject(); const value = localObject?.[key]; if ( @@ -70,12 +79,10 @@ const _setChartSetting = ( setting: PerformanceWidgetSetting ) => { const key = getContainerKey(index, performanceType, height); - const localObject = JSON.parse( - localStorage.getItem(getContainerLocalStorageObjectKey) || '{}' - ); + const localObject = getWidgetStorageObject(); localObject[key] = setting; - localStorage.setItem(getContainerLocalStorageObjectKey, JSON.stringify(localObject)); + setWidgetStorageObject(localObject); }; const _WidgetContainer = (props: Props) => { From b9d5a92a0f67734cbaa905c95ba3ae5a08da5d90 Mon Sep 17 00:00:00 2001 From: k-fish Date: Tue, 26 Oct 2021 12:26:14 -0400 Subject: [PATCH 4/4] Fix import in test --- .../performance/landing/widgets/widgetContainer.spec.jsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx b/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx index 4afe8100038d96..1d0bbb0e993fdd 100644 --- a/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx +++ b/tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx @@ -1,7 +1,7 @@ import {mountWithTheme} from 'sentry-test/enzyme'; import {initializeData} from 'sentry-test/performance/initializePerformanceData'; -import {CurrentPerformanceViewProvider} from 'app/utils/performance/contexts/currentPerformanceView'; +import {PerformanceDisplayProvider} from 'app/utils/performance/contexts/performanceDisplayContext'; import {OrganizationContext} from 'app/views/organizationContext'; import WidgetContainer from 'app/views/performance/landing/widgets/components/widgetContainer'; import {PerformanceWidgetSetting} from 'app/views/performance/landing/widgets/widgetDefinitions'; @@ -9,9 +9,7 @@ import {PROJECT_PERFORMANCE_TYPE} from 'app/views/performance/utils'; const WrappedComponent = ({data, ...rest}) => { return ( - + { forceDefaultChartSetting /> - + ); };