Skip to content

Commit dbc25be

Browse files
Merge pull request #473 from PeterYurkovich/ou-930
OU-930: use non-legacy store for accurate hideGraphs value
2 parents a3013ee + 9478adb commit dbc25be

5 files changed

Lines changed: 22 additions & 16 deletions

File tree

web/src/actions/observe.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export enum ActionType {
2929
QueryBrowserToggleAllSeries = 'queryBrowserToggleAllSeries',
3030
SetAlertCount = 'SetAlertCount',
3131
ToggleGraphs = 'toggleGraphs',
32+
ShowGraphs = 'showGraphs',
3233
SetIncidents = 'setIncidents',
3334
SetIncidentsActiveFilters = 'setIncidentsActiveFilters',
3435
SetChooseIncident = 'setChooseIncident',
@@ -93,6 +94,8 @@ export const alertingSetRules = (key: rulesKey, rules: Rule[], perspective: Pers
9394

9495
export const toggleGraphs = () => action(ActionType.ToggleGraphs);
9596

97+
export const showGraphs = () => action(ActionType.ShowGraphs);
98+
9699
export const queryBrowserAddQuery = () => action(ActionType.QueryBrowserAddQuery);
97100

98101
export const queryBrowserDuplicateQuery = (index: number) =>
@@ -186,6 +189,7 @@ const actions = {
186189
queryBrowserToggleSeries,
187190
setAlertCount,
188191
toggleGraphs,
192+
showGraphs,
189193
setIncidents,
190194
setIncidentsActiveFilters,
191195
setChooseIncident,

web/src/components/MetricsPage.tsx

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import {
7373
queryBrowserToggleAllSeries,
7474
queryBrowserToggleIsEnabled,
7575
queryBrowserToggleSeries,
76+
showGraphs,
7677
toggleGraphs,
7778
} from '../actions/observe';
7879

@@ -94,7 +95,7 @@ import {
9495
import { MonitoringState } from '../reducers/observe';
9596
import { DropDownPollInterval } from './dropdown-poll-interval';
9697
import { useBoolean } from './hooks/useBoolean';
97-
import { getLegacyObserveState, usePerspective } from './hooks/usePerspective';
98+
import { getLegacyObserveState, getObserveState, usePerspective } from './hooks/usePerspective';
9899
import KebabDropdown from './kebab-dropdown';
99100
import { colors, Error, QueryBrowser } from './query-browser';
100101
import { QueryParams } from './query-params';
@@ -321,22 +322,19 @@ export const ToggleGraph: React.FC = () => {
321322
const { perspective } = usePerspective();
322323

323324
const hideGraphs = useSelector(
324-
(state: MonitoringState) => !!getLegacyObserveState(perspective, state)?.get('hideGraphs'),
325+
(state: MonitoringState) => !!getObserveState(perspective, state)?.get('hideGraphs'),
325326
);
326327

327328
const dispatch = useDispatch();
328329
const toggle = React.useCallback(() => dispatch(toggleGraphs()), [dispatch]);
330+
329331
// Use an empty useEffect to get access to the cleanup function so that if graphs are
330-
// currently hidden then we toggle one more time as we unmount
331-
React.useEffect(
332-
() => () => {
333-
if (hideGraphs) {
334-
toggle();
335-
}
336-
},
337-
// eslint-disable-next-line react-hooks/exhaustive-deps
338-
[hideGraphs],
339-
);
332+
// currently hidden then we show the graphs as we unmount
333+
React.useEffect(() => {
334+
return () => {
335+
dispatch(showGraphs());
336+
};
337+
}, [dispatch]);
340338

341339
const icon = hideGraphs ? <ChartLineIcon /> : <CompressIcon />;
342340

@@ -985,7 +983,7 @@ const QueryBrowserWrapper: React.FC<{
985983
const dispatch = useDispatch();
986984

987985
const hideGraphs = useSelector(
988-
(state: MonitoringState) => !!getLegacyObserveState(perspective, state)?.get('hideGraphs'),
986+
(state: MonitoringState) => !!getObserveState(perspective, state)?.get('hideGraphs'),
989987
);
990988
const queriesList = useSelector((state: MonitoringState) =>
991989
getLegacyObserveState(perspective, state)?.getIn(['queryBrowser', 'queries']),

web/src/components/alerting/AlertsDetailsPage.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
getAlertsUrl,
2323
getLegacyObserveState,
2424
getNewSilenceAlertUrl,
25+
getObserveState,
2526
getRuleUrl,
2627
usePerspective,
2728
} from '../hooks/usePerspective';
@@ -99,7 +100,7 @@ const AlertsDetailsPage_: React.FC = () => {
99100
const [namespace] = useActiveNamespace();
100101

101102
const hideGraphs = useSelector(
102-
(state: MonitoringState) => !!getLegacyObserveState(perspective, state)?.get('hideGraphs'),
103+
(state: MonitoringState) => !!getObserveState(perspective, state)?.get('hideGraphs'),
103104
);
104105

105106
const alerts: Alerts = useSelector((state: MonitoringState) =>

web/src/components/query-browser.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import { queryBrowserTheme } from './query-browser-theme';
6969
import { PrometheusAPIError, TimeRange } from './types';
7070
import { getTimeRanges } from './utils';
7171

72-
import { getLegacyObserveState, usePerspective } from './hooks/usePerspective';
72+
import { getLegacyObserveState, getObserveState, usePerspective } from './hooks/usePerspective';
7373
import { MonitoringState } from '../reducers/observe';
7474
import { LoadingInline } from './console/console-shared/src/components/loading/LoadingInline';
7575
import {
@@ -590,7 +590,7 @@ const QueryBrowser_: React.FC<QueryBrowserProps> = ({
590590
const { perspective } = usePerspective();
591591

592592
const hideGraphs = useSelector(
593-
(state: MonitoringState) => !!getLegacyObserveState(perspective, state)?.get('hideGraphs'),
593+
(state: MonitoringState) => !!getObserveState(perspective, state)?.get('hideGraphs'),
594594
);
595595
const tickInterval = useSelector(
596596
(state: MonitoringState) =>

web/src/reducers/observe.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,9 @@ export default (state: ObserveState, action: ObserveAction): ObserveState => {
202202
case ActionType.ToggleGraphs:
203203
return state.set('hideGraphs', !state.get('hideGraphs'));
204204

205+
case ActionType.ShowGraphs:
206+
return state.set('hideGraphs', false);
207+
205208
case ActionType.QueryBrowserAddQuery:
206209
return state.setIn(
207210
['queryBrowser', 'queries'],

0 commit comments

Comments
 (0)