Skip to content

Commit

Permalink
ref(dashboards): Remove Widget Viewer minimap feature (#82824)
Browse files Browse the repository at this point in the history
This is effectively a revert of
#34079. We never shipped that
feature, and don't have plans to do so.

This PR reverts the major changes from that part:

1. Passing and handling `showSlider` around the Dashboard widgets and
the Widget Viewer modal
2. The `chartZoomOptions` prop that's only used in the Widget Viewer is
gone from both the component and hook version of chart zoom
3. The widget viewer modal state handling is reverted back to what it
was before
4. The "augmented" zoom handler is also gone

I left the `noPadding` prop alone. It's harmless, and partially in use.
  • Loading branch information
gggritso authored Jan 3, 2025
1 parent 7a624b6 commit 0ab6cb8
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 332 deletions.
21 changes: 3 additions & 18 deletions static/app/components/charts/chartZoom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {Component} from 'react';
import type {
DataZoomComponentOption,
ECharts,
InsideDataZoomComponentOption,
ToolboxComponentOption,
XAXisComponentOption,
} from 'echarts';
Expand All @@ -11,7 +10,6 @@ import * as qs from 'query-string';

import {updateDateTime} from 'sentry/actionCreators/pageFilters';
import DataZoomInside from 'sentry/components/charts/components/dataZoomInside';
import DataZoomSlider from 'sentry/components/charts/components/dataZoomSlider';
import ToolBox from 'sentry/components/charts/components/toolBox';
import type {DateString} from 'sentry/types/core';
import type {
Expand Down Expand Up @@ -54,7 +52,6 @@ export interface ZoomRenderProps extends Pick<Props, ZoomPropKeys> {

type Props = {
children: (props: ZoomRenderProps) => React.ReactNode;
chartZoomOptions?: DataZoomComponentOption;
disabled?: boolean;
end?: DateString;
onChartReady?: EChartChartReadyHandler;
Expand All @@ -65,7 +62,6 @@ type Props = {
period?: string | null;
router?: InjectedRouter;
saveOnZoom?: boolean;
showSlider?: boolean;
start?: DateString;
usePageDate?: boolean;
utc?: boolean | null;
Expand Down Expand Up @@ -336,8 +332,6 @@ class ChartZoom extends Component<Props> {
onChartReady: _onChartReady,
onDataZoom: _onDataZoom,
onFinished: _onFinished,
showSlider,
chartZoomOptions,
...props
} = this.props;

Expand All @@ -360,18 +354,9 @@ class ChartZoom extends Component<Props> {
utc,
start,
end,
dataZoom: showSlider
? [
...DataZoomSlider({xAxisIndex, ...chartZoomOptions}),
...DataZoomInside({
xAxisIndex,
...(chartZoomOptions as InsideDataZoomComponentOption),
}),
]
: DataZoomInside({
xAxisIndex,
...(chartZoomOptions as InsideDataZoomComponentOption),
}),
dataZoom: DataZoomInside({
xAxisIndex,
}),
showTimeInTooltip: true,
toolBox: ToolBox(
{},
Expand Down
26 changes: 0 additions & 26 deletions static/app/components/charts/components/dataZoomSlider.tsx

This file was deleted.

19 changes: 3 additions & 16 deletions static/app/components/charts/useChartZoom.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
import {useCallback, useEffect, useMemo, useRef} from 'react';
import type {
DataZoomComponentOption,
ECharts,
InsideDataZoomComponentOption,
ToolboxComponentOption,
} from 'echarts';
import type {DataZoomComponentOption, ECharts, ToolboxComponentOption} from 'echarts';
import * as qs from 'query-string';

import {updateDateTime} from 'sentry/actionCreators/pageFilters';
import DataZoomInside from 'sentry/components/charts/components/dataZoomInside';
import DataZoomSlider from 'sentry/components/charts/components/dataZoomSlider';
import ToolBox from 'sentry/components/charts/components/toolBox';
import type {DateString} from 'sentry/types/core';
import type {
Expand Down Expand Up @@ -43,7 +37,6 @@ interface ZoomRenderProps {

interface Props {
children: (props: ZoomRenderProps) => React.ReactNode;
chartZoomOptions?: DataZoomComponentOption;
/**
* Disables saving changes to the current period
*/
Expand All @@ -54,7 +47,6 @@ interface Props {
* Will persist zoom state to page filters
*/
saveOnZoom?: boolean;
showSlider?: boolean;
/**
* Use either `saveOnZoom` or `usePageDate` not both
* Persists zoom state to query params without updating page filters.
Expand Down Expand Up @@ -135,8 +127,6 @@ export function useChartZoom({
usePageDate,
saveOnZoom,
xAxisIndex,
showSlider,
chartZoomOptions,
}: Omit<Props, 'children'>): ZoomRenderProps {
const {handleChartReady} = useChartZoomCancel();
const location = useLocation();
Expand Down Expand Up @@ -256,12 +246,9 @@ export function useChartZoom({
const dataZoomProp = useMemo<DataZoomComponentOption[]>(() => {
const zoomInside = DataZoomInside({
xAxisIndex,
...(chartZoomOptions as InsideDataZoomComponentOption),
});
return showSlider
? [...DataZoomSlider({xAxisIndex, ...chartZoomOptions}), ...zoomInside]
: zoomInside;
}, [chartZoomOptions, showSlider, xAxisIndex]);
return zoomInside;
}, [xAxisIndex]);

const toolBox = useMemo<ToolboxComponentOption>(
() =>
Expand Down
136 changes: 0 additions & 136 deletions static/app/components/modals/widgetViewerModal.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -443,71 +443,6 @@ describe('Modals -> WidgetViewerModal', function () {
);
});

it('renders widget chart minimap', async function () {
initialData.organization.features.push('widget-viewer-modal-minimap');
mockEvents();
await renderModal({
initialData,
widget: {
...mockWidget,
queries: [{...mockQuery, name: ''}, additionalMockQuery],
},
});

expect(ReactEchartsCore).toHaveBeenLastCalledWith(
expect.objectContaining({
option: expect.objectContaining({
dataZoom: expect.arrayContaining([
expect.objectContaining({
realtime: false,
showDetail: false,
end: 100,
start: 0,
}),
]),
}),
}),
{}
);
});

it('zooming on minimap updates location query and updates echart start and end values', async function () {
initialData.organization.features.push('widget-viewer-modal-minimap');
mockEvents();
await renderModal({
initialData,
widget: {
...mockWidget,
queries: [{...mockQuery, name: ''}, additionalMockQuery],
},
});
const calls = (ReactEchartsCore as jest.Mock).mock.calls;
act(() => {
// Simulate dataZoom event on chart
calls[calls.length - 1][0].onEvents.datazoom(
{seriesStart: 1646100000000, seriesEnd: 1646120000000},
{
getModel: () => {
return {
_payload: {start: 30, end: 70},
};
},
}
);
});

await waitFor(() =>
expect(initialData.router.push).toHaveBeenCalledWith(
expect.objectContaining({
query: {
viewerEnd: '2022-03-01T05:53:20',
viewerStart: '2022-03-01T03:40:00',
},
})
)
);
});

it('includes group by in widget viewer table', async function () {
mockEvents();
mockWidget.queries = [
Expand Down Expand Up @@ -912,77 +847,6 @@ describe('Modals -> WidgetViewerModal', function () {
await waitForMetaToHaveBeenCalled();
expect(eventsStatsMock).toHaveBeenCalledTimes(1);
});

it('renders widget chart minimap', async function () {
mockEventsStats();
mockEvents();
initialData.organization.features.push('widget-viewer-modal-minimap');
await renderModal({initialData, widget: mockWidget});

expect(ReactEchartsCore).toHaveBeenLastCalledWith(
expect.objectContaining({
option: expect.objectContaining({
dataZoom: expect.arrayContaining([
expect.objectContaining({
realtime: false,
showDetail: false,
end: 100,
start: 0,
}),
]),
}),
}),
{}
);
});

it('zooming on minimap updates location query and updates echart start and end values', async function () {
mockEventsStats();
mockEvents();
initialData.organization.features.push('widget-viewer-modal-minimap');
await renderModal({initialData, widget: mockWidget});
const calls = (ReactEchartsCore as jest.Mock).mock.calls;
act(() => {
// Simulate dataZoom event on chart
calls[calls.length - 1][0].onEvents.datazoom(
{seriesStart: 1646100000000, seriesEnd: 1646120000000},
{
getModel: () => {
return {
_payload: {start: 30, end: 70},
};
},
}
);
});

expect(initialData.router.push).toHaveBeenCalledWith(
expect.objectContaining({
query: {
viewerEnd: '2022-03-01T05:53:20',
viewerStart: '2022-03-01T03:40:00',
},
})
);

await waitFor(() => {
expect(ReactEchartsCore).toHaveBeenLastCalledWith(
expect.objectContaining({
option: expect.objectContaining({
dataZoom: expect.arrayContaining([
expect.objectContaining({
realtime: false,
showDetail: false,
endValue: 1646114000000,
startValue: 1646106000000,
}),
]),
}),
}),
{}
);
});
});
});

describe('Table Widget', function () {
Expand Down
Loading

0 comments on commit 0ab6cb8

Please sign in to comment.