Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(insights): Use GlobalDrawer instead of detail panels #82534

Merged
merged 33 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
c1663c3
Add panel header component
gggritso Dec 20, 2024
432408b
Add body component
gggritso Dec 20, 2024
fc60207
Truncate the link
gggritso Dec 20, 2024
b39998d
Use new drawer in HTTP samples panel
gggritso Dec 20, 2024
4c72a84
Extract drawer hook
gggritso Dec 20, 2024
d0aee68
Use new drawer in Caches
gggritso Dec 20, 2024
2c30adb
Use new Drawer in Queues
gggritso Dec 20, 2024
080527b
Pass through a React node instead
gggritso Dec 20, 2024
51b42a3
Omit query when closing the drawer
gggritso Dec 20, 2024
b7a892c
Use new Drawer in Queries and Resources
gggritso Dec 20, 2024
3e54ef6
Custon `onClose`
gggritso Dec 23, 2024
102b9ae
Revert "Custon `onClose`"
gggritso Dec 23, 2024
80cffdd
Use drawer in Web Vitals
gggritso Dec 23, 2024
76dc8c6
Give vitals drawer a more specific name
gggritso Dec 23, 2024
cf66ff3
Omit more span properties from URL on close
gggritso Dec 23, 2024
8a92f4f
Use drawer in Mobile
gggritso Dec 23, 2024
79ca100
Fold me into rename
gggritso Dec 23, 2024
c901671
Project is optional for rendering the drawer header
gggritso Dec 23, 2024
4550c7d
Use heading for header
gggritso Dec 23, 2024
3b5a93f
Do not gate header behind project
gggritso Dec 23, 2024
54842f2
Fix data loading in cache panel
gggritso Dec 23, 2024
8b799a0
Add subtitle support to drawer header
gggritso Dec 23, 2024
7539a2c
Add subtitle to queue panels
gggritso Dec 23, 2024
99ca171
Remove unneeded test
gggritso Dec 23, 2024
4521aec
Rename bad interface
gggritso Dec 23, 2024
af53630
Remove unused prop
gggritso Dec 23, 2024
29fa975
Fix bad truncations
gggritso Dec 23, 2024
1db9c81
Fix analytics
gggritso Dec 23, 2024
62dc39c
Merge branch 'master' into feat/insights/new-slideout-panel-design
gggritso Jan 2, 2025
59de2b5
Merge branch 'master' into feat/insights/new-slideout-panel-design
gggritso Jan 6, 2025
db0be36
Rename over-specified component
gggritso Jan 6, 2025
f443bd2
Add subtitle delimiter
gggritso Jan 6, 2025
d134cc5
Update specs
gggritso Jan 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {useEffect} from 'react';

import * as Layout from 'sentry/components/layouts/thirds';
import {t, tct} from 'sentry/locale';
Expand All @@ -23,6 +23,7 @@ import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/modu
import {ToolRibbon} from 'sentry/views/insights/common/components/ribbon';
import {useSpanMetrics} from 'sentry/views/insights/common/queries/useDiscover';
import {useModuleURL} from 'sentry/views/insights/common/utils/useModuleURL';
import {useSamplesDrawer} from 'sentry/views/insights/common/utils/useSamplesDrawer';
import SubregionSelector from 'sentry/views/insights/common/views/spans/selectors/subregionSelector';
import {SampleList} from 'sentry/views/insights/common/views/spanSummaryPage/sampleList';
import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
Expand All @@ -47,6 +48,27 @@ function ResourceSummary() {
const {
query: {transaction},
} = useLocation();

const {openSamplesDrawer} = useSamplesDrawer({
Component: (
<SampleList
transactionRoute={webVitalsModuleURL}
subregions={filters[SpanMetricsField.USER_GEO_SUBREGION]}
groupId={groupId}
moduleName={ModuleName.RESOURCE}
transactionName={transaction as string}
referrer={TraceViewSources.ASSETS_MODULE}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Maybe we could use decodeScalar so we don't need this type assertion? might have to handle undefined though or add a fallback

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably yes! I didn't bother changing this since it's not part of the panel change (I'm just moving the code around) but I agree that using decodeScalar (probably in useLocationQuery) would be better here

/>
),
moduleName: ModuleName.RESOURCE,
});

useEffect(() => {
if (transaction) {
openSamplesDrawer();
}
});

const {data, isPending} = useSpanMetrics(
{
search: MutableSearch.fromQueryObject({
Expand Down Expand Up @@ -136,17 +158,6 @@ function ResourceSummary() {
<ModuleLayout.Full>
<ResourceSummaryTable />
</ModuleLayout.Full>

<ModuleLayout.Full>
<SampleList
transactionRoute={webVitalsModuleURL}
subregions={filters[SpanMetricsField.USER_GEO_SUBREGION]}
groupId={groupId}
moduleName={ModuleName.RESOURCE}
transactionName={transaction as string}
referrer={TraceViewSources.ASSETS_MODULE}
/>
</ModuleLayout.Full>
</ModuleLayout.Layout>
</Layout.Main>
</Layout.Body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {useMemo} from 'react';
import styled from '@emotion/styled';

import type {LineChartSeries} from 'sentry/components/charts/lineChart';
import {DrawerHeader} from 'sentry/components/globalDrawer/components';
import type {
GridColumnHeader,
GridColumnOrder,
Expand Down Expand Up @@ -38,7 +39,7 @@ import type {
} from 'sentry/views/insights/browser/webVitals/types';
import decodeBrowserTypes from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
import useProfileExists from 'sentry/views/insights/browser/webVitals/utils/useProfileExists';
import DetailPanel from 'sentry/views/insights/common/components/detailPanel';
import {SampleDrawerBody} from 'sentry/views/insights/common/components/sampleDrawerBody';
import {SpanIndexedField, type SubregionCode} from 'sentry/views/insights/types';
import {TraceViewSources} from 'sentry/views/performance/newTraceDetails/traceHeader/breadcrumbs';
import {generateReplayLink} from 'sentry/views/performance/transactionSummary/utils';
Expand Down Expand Up @@ -77,9 +78,7 @@ const inpSort: GridColumnSortBy<keyof InteractionSpanSampleRowWithScore> = {

export function PageOverviewWebVitalsDetailPanel({
webVital,
onClose,
}: {
onClose: () => void;
webVital: WebVitals | null;
}) {
const location = useLocation();
Expand Down Expand Up @@ -369,7 +368,9 @@ export function PageOverviewWebVitalsDetailPanel({

return (
<PageAlertProvider>
<DetailPanel detailKey={webVital ?? undefined} onClose={onClose}>
<DrawerHeader />

<SampleDrawerBody>
{webVital && (
<WebVitalDetailHeader
value={
Expand Down Expand Up @@ -412,7 +413,7 @@ export function PageOverviewWebVitalsDetailPanel({
)}
</TableContainer>
<PageAlert />
</DetailPanel>
</SampleDrawerBody>
</PageAlertProvider>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ const Value = styled('h2')`

const WebVitalName = styled('h4')`
margin-bottom: ${space(1)};
margin-top: 40px;
max-width: 400px;
${p => p.theme.overflowEllipsis}
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('WebVitalsDetailPanel', function () {
});

it('renders correctly with empty results', async () => {
render(<WebVitalsDetailPanel onClose={() => undefined} webVital="lcp" />, {
render(<WebVitalsDetailPanel webVital="lcp" />, {
organization,
});
await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {useEffect, useMemo} from 'react';
import styled from '@emotion/styled';

import type {LineChartSeries} from 'sentry/components/charts/lineChart';
import {DrawerHeader} from 'sentry/components/globalDrawer/components';
import type {
GridColumnHeader,
GridColumnOrder,
Expand Down Expand Up @@ -34,7 +35,7 @@ import type {
WebVitals,
} from 'sentry/views/insights/browser/webVitals/types';
import decodeBrowserTypes from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
import DetailPanel from 'sentry/views/insights/common/components/detailPanel';
import {SampleDrawerBody} from 'sentry/views/insights/common/components/sampleDrawerBody';
import {SpanIndexedField, type SubregionCode} from 'sentry/views/insights/types';

type Column = GridColumnHeader;
Expand All @@ -51,13 +52,7 @@ const sort: GridColumnSortBy<keyof Row> = {key: 'count()', order: 'desc'};

const MAX_ROWS = 10;

export function WebVitalsDetailPanel({
webVital,
onClose,
}: {
onClose: () => void;
webVital: WebVitals | null;
}) {
export function WebVitalsDetailPanel({webVital}: {webVital: WebVitals | null}) {
const location = useLocation();
const organization = useOrganization();
const browserTypes = decodeBrowserTypes(location.query[SpanIndexedField.BROWSER_NAME]);
Expand Down Expand Up @@ -135,8 +130,6 @@ export function WebVitalsDetailPanel({
seriesName: webVital ?? '',
};

const detailKey = webVital;

useEffect(() => {
if (webVital !== null) {
trackAnalytics('insight.vital.vital_sidebar_opened', {
Expand Down Expand Up @@ -245,7 +238,9 @@ export function WebVitalsDetailPanel({

return (
<PageAlertProvider>
<DetailPanel detailKey={detailKey ?? undefined} onClose={onClose}>
<DrawerHeader />

<SampleDrawerBody>
{webVital && (
<WebVitalDescription
value={
Expand Down Expand Up @@ -276,7 +271,7 @@ export function WebVitalsDetailPanel({
/>
</TableContainer>
<PageAlert />
</DetailPanel>
</SampleDrawerBody>
</PageAlertProvider>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {Fragment, useMemo, useState} from 'react';
import React, {Fragment, useEffect, useMemo, useState} from 'react';
import styled from '@emotion/styled';
import omit from 'lodash/omit';

Expand Down Expand Up @@ -32,6 +32,7 @@ import {ModulePageFilterBar} from 'sentry/views/insights/common/components/modul
import {ModulePageProviders} from 'sentry/views/insights/common/components/modulePageProviders';
import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/moduleUpsellHookWrapper';
import {useModuleURL} from 'sentry/views/insights/common/utils/useModuleURL';
import {useWebVitalsDrawer} from 'sentry/views/insights/common/utils/useWebVitalsDrawer';
import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
import {useDomainViewFilters} from 'sentry/views/insights/pages/useFilters';
import {
Expand Down Expand Up @@ -105,6 +106,25 @@ export function PageOverview() {
const {data: projectScores, isPending: isProjectScoresLoading} =
useProjectWebVitalsScoresQuery({transaction, browserTypes, subregions});

const {openVitalsDrawer} = useWebVitalsDrawer({
Component: <PageOverviewWebVitalsDetailPanel webVital={state.webVital} />,
webVital: state.webVital,
onClose: () => {
router.replace({
pathname: router.location.pathname,
query: omit(router.location.query, 'webVital'),
});

setState({...state, webVital: null});
},
});

useEffect(() => {
if (state.webVital) {
openVitalsDrawer();
}
});

if (transaction === undefined) {
// redirect user to webvitals landing page
window.location.href = moduleURL;
Expand Down Expand Up @@ -238,16 +258,6 @@ export function PageOverview() {
</Layout.Body>
)}
</ModuleBodyUpsellHook>
<PageOverviewWebVitalsDetailPanel
webVital={state.webVital}
onClose={() => {
router.replace({
pathname: router.location.pathname,
query: omit(router.location.query, 'webVital'),
});
setState({...state, webVital: null});
}}
/>
</Tabs>
</React.Fragment>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, {Fragment, useState} from 'react';
import React, {Fragment, useEffect, useState} from 'react';
import styled from '@emotion/styled';
import omit from 'lodash/omit';

import Alert from 'sentry/components/alert';
import {Button} from 'sentry/components/button';
Expand All @@ -12,7 +11,6 @@ import {t, tct} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {decodeList} from 'sentry/utils/queryString';
import {useLocation} from 'sentry/utils/useLocation';
import useRouter from 'sentry/utils/useRouter';
import BrowserTypeSelector from 'sentry/views/insights/browser/webVitals/components/browserTypeSelector';
import {PerformanceScoreChart} from 'sentry/views/insights/browser/webVitals/components/charts/performanceScoreChart';
import {PagePerformanceTable} from 'sentry/views/insights/browser/webVitals/components/tables/pagePerformanceTable';
Expand All @@ -32,6 +30,7 @@ import {ModulePageFilterBar} from 'sentry/views/insights/common/components/modul
import {ModulePageProviders} from 'sentry/views/insights/common/components/modulePageProviders';
import {ModulesOnboarding} from 'sentry/views/insights/common/components/modulesOnboarding';
import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/moduleUpsellHookWrapper';
import {useWebVitalsDrawer} from 'sentry/views/insights/common/utils/useWebVitalsDrawer';
import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
import {
ModuleName,
Expand All @@ -44,8 +43,6 @@ const WEB_VITALS_COUNT = 5;
export function WebVitalsLandingPage() {
const location = useLocation();

const router = useRouter();

const [state, setState] = useState<{webVital: WebVitals | null}>({
webVital: (location.query.webVital as WebVitals) ?? null,
});
Expand All @@ -67,6 +64,20 @@ export function WebVitalsLandingPage() {
? undefined
: calculatePerformanceScoreFromStoredTableDataRow(projectScores?.data?.[0]);

const {openVitalsDrawer} = useWebVitalsDrawer({
Component: <WebVitalsDetailPanel webVital={state.webVital} />,
webVital: state.webVital,
onClose: () => {
setState({webVital: null});
},
});

useEffect(() => {
if (state.webVital) {
openVitalsDrawer();
}
});

return (
<React.Fragment>
<FrontendHeader
Expand Down Expand Up @@ -147,16 +158,6 @@ export function WebVitalsLandingPage() {
</Layout.Main>
</Layout.Body>
</ModuleBodyUpsellHook>
<WebVitalsDetailPanel
webVital={state.webVital}
onClose={() => {
router.replace({
pathname: router.location.pathname,
query: omit(router.location.query, 'webVital'),
});
setState({...state, webVital: null});
}}
/>
</React.Fragment>
);
}
Expand Down
Loading
Loading