Skip to content

Commit 95bebdf

Browse files
ahmadshaheershivanshuraj1333
authored andcommitted
refactor: address review comments
1 parent 9133683 commit 95bebdf

File tree

5 files changed

+103
-88
lines changed

5 files changed

+103
-88
lines changed

frontend/src/components/CeleryOverview/CeleryOverviewConfigOptions/CeleryOverviewConfigOptions.tsx

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useCeleryFilterOptions } from 'components/CeleryTask/useCeleryFilterOpt
99
import { SelectMaxTagPlaceholder } from 'components/MessagingQueues/MQCommon/MQCommon';
1010
import { QueryParams } from 'constants/query';
1111
import useUrlQuery from 'hooks/useUrlQuery';
12+
import { useCallback } from 'react';
1213
import { useHistory, useLocation } from 'react-router-dom';
1314

1415
export interface SelectOptionConfig {
@@ -38,6 +39,37 @@ export function FilterSelect({
3839
const history = useHistory();
3940
const location = useLocation();
4041

42+
// Use externally provided `values` if `shouldSetQueryParams` is false, otherwise get from URL params.
43+
const selectValue =
44+
!shouldSetQueryParams && !!values?.length
45+
? values
46+
: getValuesFromQueryParams(queryParam, urlQuery) || [];
47+
48+
const handleSelectChange = useCallback(
49+
(value: string | string[]): void => {
50+
handleSearch('');
51+
if (shouldSetQueryParams) {
52+
setQueryParamsFromOptions(
53+
value as string[],
54+
urlQuery,
55+
history,
56+
location,
57+
queryParam,
58+
);
59+
}
60+
onChange?.(value);
61+
},
62+
[
63+
handleSearch,
64+
shouldSetQueryParams,
65+
urlQuery,
66+
history,
67+
location,
68+
queryParam,
69+
onChange,
70+
],
71+
);
72+
4173
return (
4274
<Select
4375
key={filterType.toString()}
@@ -52,11 +84,7 @@ export function FilterSelect({
5284
maxTagCount={4}
5385
allowClear
5486
maxTagPlaceholder={SelectMaxTagPlaceholder}
55-
value={
56-
!shouldSetQueryParams && !!values?.length
57-
? values
58-
: getValuesFromQueryParams(queryParam, urlQuery) || []
59-
}
87+
value={selectValue}
6088
notFoundContent={
6189
isFetching ? (
6290
<span>
@@ -66,19 +94,7 @@ export function FilterSelect({
6694
<span>No {placeholder} found</span>
6795
)
6896
}
69-
onChange={(value): void => {
70-
handleSearch('');
71-
if (shouldSetQueryParams) {
72-
setQueryParamsFromOptions(
73-
value as string[],
74-
urlQuery,
75-
history,
76-
location,
77-
queryParam,
78-
);
79-
}
80-
onChange?.(value);
81-
}}
97+
onChange={handleSelectChange}
8298
/>
8399
);
84100
}

frontend/src/container/TraceWaterfall/TraceWaterfallStates/Success/Success.tsx

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -149,28 +149,30 @@ function SpanOverview({
149149
<Typography.Text className="service-name">
150150
{span.serviceName}
151151
</Typography.Text>
152-
{!!span.serviceName && !!span.name && (
153-
<div className="add-funnel-button">
154-
<span className="add-funnel-button__separator">·</span>
155-
<Button
156-
type="text"
157-
size="small"
158-
className="add-funnel-button__button"
159-
onClick={(e): void => {
160-
e.preventDefault();
161-
e.stopPropagation();
162-
handleAddSpanToFunnel(span);
163-
}}
164-
icon={
165-
<img
166-
className="add-funnel-button__icon"
167-
src="/Icons/funnel-add.svg"
168-
alt="funnel-icon"
169-
/>
170-
}
171-
/>
172-
</div>
173-
)}
152+
{!!span.serviceName &&
153+
!!span.name &&
154+
process.env.NODE_ENV === 'development' && (
155+
<div className="add-funnel-button">
156+
<span className="add-funnel-button__separator">·</span>
157+
<Button
158+
type="text"
159+
size="small"
160+
className="add-funnel-button__button"
161+
onClick={(e): void => {
162+
e.preventDefault();
163+
e.stopPropagation();
164+
handleAddSpanToFunnel(span);
165+
}}
166+
icon={
167+
<img
168+
className="add-funnel-button__icon"
169+
src="/Icons/funnel-add.svg"
170+
alt="funnel-icon"
171+
/>
172+
}
173+
/>
174+
</div>
175+
)}
174176
</section>
175177
</div>
176178
</div>

frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelBreadcrumb.tsx

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,24 @@ interface FunnelBreadcrumbProps {
99
}
1010

1111
function FunnelBreadcrumb({ funnelName }: FunnelBreadcrumbProps): JSX.Element {
12+
const breadcrumbItems = [
13+
{
14+
title: (
15+
<Link to={ROUTES.TRACES_FUNNELS}>
16+
<span className="funnel-breadcrumb__link">
17+
<span className="funnel-breadcrumb__title">All funnels</span>
18+
</span>
19+
</Link>
20+
),
21+
},
22+
{
23+
title: <div className="funnel-breadcrumb__title">{funnelName}</div>,
24+
},
25+
];
26+
1227
return (
1328
<div>
14-
<Breadcrumb
15-
className="funnel-breadcrumb"
16-
items={[
17-
{
18-
title: (
19-
<Link to={ROUTES.TRACES_FUNNELS}>
20-
<span className="funnel-breadcrumb__link">
21-
<span className="funnel-breadcrumb__title">All funnels</span>
22-
</span>
23-
</Link>
24-
),
25-
},
26-
{
27-
title: <div className="funnel-breadcrumb__title">{funnelName}</div>,
28-
},
29-
]}
30-
/>
29+
<Breadcrumb className="funnel-breadcrumb" items={breadcrumbItems} />
3130
</div>
3231
);
3332
}

frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelTopTracesTable.tsx

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import { ErrorTraceData, SlowTraceData } from 'api/traceFunnels';
2-
import { getYAxisFormattedValue } from 'components/Graph/yAxisConfig';
32
import { useFunnelContext } from 'pages/TracesFunnels/FunnelContext';
43
import { useMemo } from 'react';
54
import { UseQueryResult } from 'react-query';
6-
import { Link } from 'react-router-dom';
75
import { ErrorResponse, SuccessResponse } from 'types/api';
86

97
import FunnelTable from './FunnelTable';
8+
import { topTracesTableColumns } from './utils';
109

1110
interface FunnelTopTracesTableProps {
1211
funnelId: string;
@@ -61,39 +60,11 @@ function FunnelTopTracesTable({
6160
}));
6261
}, [response]);
6362

64-
const columns = useMemo(
65-
() => [
66-
{
67-
title: 'TRACE ID',
68-
dataIndex: 'trace_id',
69-
key: 'trace_id',
70-
render: (traceId: string): JSX.Element => (
71-
<Link to={`/trace/${traceId}`} className="trace-id-cell">
72-
{traceId}
73-
</Link>
74-
),
75-
},
76-
{
77-
title: 'DURATION',
78-
dataIndex: 'duration_ms',
79-
key: 'duration_ms',
80-
render: (value: string): string => getYAxisFormattedValue(value, 'ms'),
81-
},
82-
{
83-
title: 'SPAN COUNT',
84-
dataIndex: 'span_count',
85-
key: 'span_count',
86-
render: (value: number): string => value.toString(),
87-
},
88-
],
89-
[],
90-
);
91-
9263
return (
9364
<FunnelTable
9465
title={title}
9566
tooltip={tooltip}
96-
columns={columns}
67+
columns={topTracesTableColumns}
9768
data={data}
9869
loading={isLoading || isFetching}
9970
/>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { getYAxisFormattedValue } from 'components/Graph/yAxisConfig';
2+
import { Link } from 'react-router-dom';
3+
4+
export const topTracesTableColumns = [
5+
{
6+
title: 'TRACE ID',
7+
dataIndex: 'trace_id',
8+
key: 'trace_id',
9+
render: (traceId: string): JSX.Element => (
10+
<Link to={`/trace/${traceId}`} className="trace-id-cell">
11+
{traceId}
12+
</Link>
13+
),
14+
},
15+
{
16+
title: 'DURATION',
17+
dataIndex: 'duration_ms',
18+
key: 'duration_ms',
19+
render: (value: string): string => getYAxisFormattedValue(value, 'ms'),
20+
},
21+
{
22+
title: 'SPAN COUNT',
23+
dataIndex: 'span_count',
24+
key: 'span_count',
25+
render: (value: number): string => value.toString(),
26+
},
27+
];

0 commit comments

Comments
 (0)