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.

Sign up for GitHub

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(perf): Finish remaining landing v3 views #29715

Merged
merged 6 commits into from
Nov 2, 2021
Merged

feat(perf): Finish remaining landing v3 views #29715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
79f10d1
feat(perf): Finish remaining landing v3 views
k-fish Nov 1, 2021
c256c1a
Fix issues with memoization and requests refiring when event view cha…
k-fish Nov 1, 2021
c80c831
Fix tsc
k-fish Nov 2, 2021
5edac02
Update static/app/views/performance/landing/widgets/widgetDefinitions…
k-fish Nov 2, 2021
60e8cd8
Pass in eventView from props now that it's now not always provided
k-fish Nov 2, 2021
88bf69c
Merge branch 'feat/perf-landing-finish-views-2' of github.com:getsent…
k-fish Nov 2, 2021
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
41 changes: 31 additions & 10 deletions static/app/views/performance/charts/chart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import min from 'lodash/min';

import AreaChart from 'app/components/charts/areaChart';
import ChartZoom from 'app/components/charts/chartZoom';
import LineChart from 'app/components/charts/lineChart';
import {DateString} from 'app/types';
import {Series} from 'app/types/echarts';
import {axisLabelFormatter, tooltipFormatter} from 'app/utils/discover/charts';
Expand All @@ -24,6 +25,7 @@ type Props = {
disableXAxis?: boolean;
chartColors?: string[];
loading: boolean;
isLineChart?: boolean;
};

// adapted from https://stackoverflow.com/questions/11397239/rounding-up-for-a-graph-maximum
Expand Down Expand Up @@ -68,6 +70,7 @@ function Chart({
disableMultiAxis,
disableXAxis,
chartColors,
isLineChart,
}: Props) {
const theme = useTheme();

Expand Down Expand Up @@ -176,6 +179,9 @@ function Chart({
};

if (loading) {
if (isLineChart) {
return <LineChart height={height} series={[]} {...areaChartProps} />;
}
Comment on lines +182 to +184
Copy link
Member

Choose a reason for hiding this comment

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

Can be a follow-up, but I think we typically use the TransparentLoadingMask over a chart to indicate loading states.

Copy link
Member Author

Choose a reason for hiding this comment

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

The transparent loading mask should only appear on reload state, which I'm thinking of handling in performance widget under the hood, instead of in these charts.

return <AreaChart height={height} series={[]} {...areaChartProps} />;
}
const series = data.map((values, i: number) => ({
Expand All @@ -193,16 +199,31 @@ function Chart({
utc={utc}
xAxisIndex={disableMultiAxis ? undefined : [0, 1]}
>
{zoomRenderProps => (
<AreaChart
height={height}
{...zoomRenderProps}
series={series}
previousPeriod={previousData}
xAxis={disableXAxis ? {show: false} : undefined}
{...areaChartProps}
/>
)}
{zoomRenderProps => {
if (isLineChart) {
return (
<LineChart
height={height}
{...zoomRenderProps}
series={series}
previousPeriod={previousData}
xAxis={disableXAxis ? {show: false} : undefined}
{...areaChartProps}
/>
);
}

return (
<AreaChart
height={height}
{...zoomRenderProps}
series={series}
previousPeriod={previousData}
xAxis={disableXAxis ? {show: false} : undefined}
{...areaChartProps}
/>
);
}}
</ChartZoom>
);
}
Expand Down
2 changes: 2 additions & 0 deletions static/app/views/performance/data.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export enum PERFORMANCE_TERM {
STALL_PERCENTAGE = 'stallPercentage',
MOST_ISSUES = 'mostIssues',
MOST_ERRORS = 'mostErrors',
SLOW_HTTP_SPANS = 'slowHTTPSpans',
}

export type TooltipOption = SelectValue<string> & {
Expand Down Expand Up @@ -361,6 +362,7 @@ export const PERFORMANCE_TERMS: Record<PERFORMANCE_TERM, TermFormatter> = {
frozenFrames: () => t('The count of the number of frozen frames in the transaction.'),
mostErrors: () => t('Transactions with the most associated errors.'),
mostIssues: () => t('The most instances of an issue for a related transaction.'),
slowHTTPSpans: () => t('The transactions with the slowest spans of a certain type.'),
Copy link
Member

Choose a reason for hiding this comment

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

Should there be custom messages for each type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Thanks for reminding me.

stallPercentage: () =>
t(
'The percentage of the transaction duration in which the application is in a stalled state.'
Expand Down
4 changes: 1 addition & 3 deletions static/app/views/performance/landing/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {Location} from 'history';

import DropdownControl, {DropdownItem} from 'app/components/dropdownControl';
import SearchBar from 'app/components/events/searchBar';
import FeatureBadge from 'app/components/featureBadge';
import * as TeamKeyTransactionManager from 'app/components/performance/teamKeyTransactionsManager';
import {MAX_QUERY_LENGTH} from 'app/constants';
import {t} from 'app/locale';
Expand Down Expand Up @@ -296,7 +295,7 @@ class LandingContent extends Component<Props, State> {
>
{LANDING_DISPLAYS.filter(
({isShown}) => !isShown || isShown(organization)
).map(({badge, label, field}) => (
).map(({label, field}) => (
<DropdownItem
key={field}
onSelect={this.handleLandingDisplayChange}
Expand All @@ -305,7 +304,6 @@ class LandingContent extends Component<Props, State> {
isActive={field === currentLandingDisplay.field}
>
{label}
{badge && <FeatureBadge type={badge} noTooltip />}
</DropdownItem>
))}
</DropdownControl>
Expand Down
4 changes: 1 addition & 3 deletions static/app/views/performance/landing/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {Location} from 'history';

import Button from 'app/components/button';
import SearchBar from 'app/components/events/searchBar';
import FeatureBadge from 'app/components/featureBadge';
import GlobalSdkUpdateAlert from 'app/components/globalSdkUpdateAlert';
import * as Layout from 'app/components/layouts/thirds';
import NavTabs from 'app/components/navTabs';
Expand Down Expand Up @@ -101,14 +100,13 @@ function _PerformanceLanding(props: Props) {
</Layout.HeaderActions>

<StyledNavTabs>
{shownLandingDisplays.map(({badge, label, field}) => (
{shownLandingDisplays.map(({label, field}) => (
<li
key={label}
className={currentLandingDisplay.field === field ? 'active' : ''}
>
<a href="#" onClick={() => handleLandingDisplayChange(field, location)}>
{t(label)}
{badge && <FeatureBadge type={badge} />}
</a>
</li>
))}
Expand Down
1 change: 0 additions & 1 deletion static/app/views/performance/landing/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const LANDING_DISPLAYS = [
field: LandingDisplayField.MOBILE,
isShown: (organization: Organization) =>
organization.features.includes('performance-mobile-vitals'),
badge: 'new' as const,
},
];

Expand Down
43 changes: 36 additions & 7 deletions static/app/views/performance/landing/views/backendView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,47 @@
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';
import {BACKEND_COLUMN_TITLES} from '../data';
import {DoubleChartRow, TripleChartRow} from '../widgets/components/widgetChartRow';
import {PerformanceWidgetSetting} from '../widgets/widgetDefinitions';

import {BasePerformanceViewProps} from './types';

export function BackendView(props: BasePerformanceViewProps) {
return (
<div>
<Table
{...props}
columnTitles={BACKEND_COLUMN_TITLES}
setError={usePageError().setPageError}
/>
</div>
<PerformanceDisplayProvider value={{performanceType: PROJECT_PERFORMANCE_TYPE.ANY}}>
<div>
<TripleChartRow
{...props}
allowedCharts={[
PerformanceWidgetSetting.APDEX_AREA,
PerformanceWidgetSetting.TPM_AREA,
PerformanceWidgetSetting.FAILURE_RATE_AREA,
PerformanceWidgetSetting.USER_MISERY_AREA,
PerformanceWidgetSetting.P50_DURATION_AREA,
PerformanceWidgetSetting.P75_DURATION_AREA,
PerformanceWidgetSetting.P95_DURATION_AREA,
PerformanceWidgetSetting.P99_DURATION_AREA,
PerformanceWidgetSetting.DURATION_HISTOGRAM,
]}
/>
<DoubleChartRow
{...props}
allowedCharts={[
PerformanceWidgetSetting.SLOW_HTTP_OPS,
PerformanceWidgetSetting.SLOW_DB_OPS,
PerformanceWidgetSetting.MOST_IMPROVED,
PerformanceWidgetSetting.MOST_REGRESSED,
]}
/>
<Table
{...props}
columnTitles={BACKEND_COLUMN_TITLES}
setError={usePageError().setPageError}
/>
</div>
</PerformanceDisplayProvider>
);
}
2 changes: 1 addition & 1 deletion static/app/views/performance/landing/views/frontendOtherView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function FrontendOtherView(props: BasePerformanceViewProps) {
<PerformanceDisplayProvider
value={{performanceType: PROJECT_PERFORMANCE_TYPE.FRONTEND_OTHER}}
>
<div data-test-id="frontend-pageload-view">
<div>
<TripleChartRow
{...props}
allowedCharts={[
Expand Down
39 changes: 32 additions & 7 deletions static/app/views/performance/landing/views/mobileView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,43 @@
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';
import {MOBILE_COLUMN_TITLES} from '../data';
import {DoubleChartRow, TripleChartRow} from '../widgets/components/widgetChartRow';
import {PerformanceWidgetSetting} from '../widgets/widgetDefinitions';

import {BasePerformanceViewProps} from './types';

export function MobileView(props: BasePerformanceViewProps) {
return (
<div>
<Table
{...props}
columnTitles={MOBILE_COLUMN_TITLES} // TODO(k-fish): Add react native column titles
setError={usePageError().setPageError}
/>
</div>
<PerformanceDisplayProvider value={{performanceType: PROJECT_PERFORMANCE_TYPE.ANY}}>
<div>
<TripleChartRow
{...props}
allowedCharts={[
PerformanceWidgetSetting.TPM_AREA,
PerformanceWidgetSetting.COLD_STARTUP_AREA,
PerformanceWidgetSetting.WARM_STARTUP_AREA,
PerformanceWidgetSetting.SLOW_FRAMES_AREA,
PerformanceWidgetSetting.FROZEN_FRAMES_AREA,
]}
/>
<DoubleChartRow
{...props}
allowedCharts={[
PerformanceWidgetSetting.MOST_SLOW_FRAMES,
PerformanceWidgetSetting.MOST_FROZEN_FRAMES,
PerformanceWidgetSetting.MOST_IMPROVED,
PerformanceWidgetSetting.MOST_REGRESSED,
]}
/>
<Table
{...props}
columnTitles={MOBILE_COLUMN_TITLES} // TODO(k-fish): Add react native column titles
setError={usePageError().setPageError}
/>
</div>
</PerformanceDisplayProvider>
);
}
2 changes: 0 additions & 2 deletions static/app/views/performance/landing/widgets/components/queryHandler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ export function QueryHandler<T extends WidgetDataConstraint>(
organization={props.queryProps.organization}
orgSlug={props.queryProps.organization.slug}
query={props.queryProps.eventView.getQueryWithAdditionalConditions()}
eventView={props.queryProps.eventView}
location={props.queryProps.location}
widgetData={props.widgetData}
>
{results => {
Expand Down
10 changes: 6 additions & 4 deletions static/app/views/performance/landing/widgets/components/widgetHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ export function WidgetHeader<T extends WidgetDataConstraint>(
return (
<WidgetHeaderContainer>
<TitleContainer>
<StyledHeaderTitleLegend data-test-id="performance-widget-title">
{title}
<QuestionTooltip position="top" size="sm" title={titleTooltip} />
</StyledHeaderTitleLegend>
<div>
<StyledHeaderTitleLegend data-test-id="performance-widget-title">
<div className="truncate">{title}</div>
<QuestionTooltip position="top" size="sm" title={titleTooltip} />
</StyledHeaderTitleLegend>
</div>
<div>{Subtitle ? <Subtitle {...props} /> : null}</div>
</TitleContainer>

Expand Down
2 changes: 0 additions & 2 deletions static/app/views/performance/landing/widgets/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ export type QueryFC<T extends WidgetDataConstraint> = FunctionComponent<
team?: Readonly<string | string[]>;
query?: string;
orgSlug: string;
location: Location;
organization: OrganizationSummary;
eventView: EventView;
widgetData: T;
}
>;
Expand Down
Loading