Skip to content

Commit

Permalink
[Synthetics UI] Fix infinite scroll in overview page (elastic#146570)
Browse files Browse the repository at this point in the history
## Summary

Closes elastic#145378.

The overview page has a [`<EuiSpacer />` at the bottom of the
grid](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L135-L137)
to track the user's scroll position. This is done [with an
`IntersectionObserver` (wrapped in a hook) and a
`ref`](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L70-L75).

Sometimes the `ref` got destroyed, and with it the
`IntersectionObserver` instance. This causes a race condition [in the
code that checks for the
intersection](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L78-L88)
to determine if the next page should be loaded or not.

<img width="1278" alt="Screenshot 2022-11-29 at 16 35 42"
src="https://user-images.githubusercontent.com/57448/204574461-5056391c-e96d-4b38-9c9e-4976e0d14a40.png">


The reason why the `ref` got destroyed was [this early
return](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L98-L100).
With the code in `main` there is a brief moment in which the condition
holds `true`, right after the monitors are loaded. In that brief instant
the `status` is present but `monitorsSortedByStatus` is empty.

<img width="1084" alt="Screenshot 2022-11-29 at 16 44 35"
src="https://user-images.githubusercontent.com/57448/204575679-de4bc6bf-122b-4c6d-ae75-9c96a7c5fb85.png">

When this happens the `ref` is destroyed, because the underlying element
used to track the intersection is destroyed due of the early return.

This was caused because `monitorsSortedByStatus` was updated
asynchronously [with
`useEffect`](https://github.com/elastic/kibana/blob/2f3313371bc6d992a99accef1289dd035779d3e6/x-pack/plugins/synthetics/public/apps/synthetics/hooks/use_monitors_sorted_by_status.tsx#L30-L72)
as part of the component lifecycle.

This PR uses `useMemo` instead of `useEffect`, to update the
`monitorsSortedByStatus` at the same time that `status` is present. This
prevents the early return from ever firing in the normal load cycle and
keeps the `ref`. Since the `ref` never gets destroyed there's no race
condition anymore.
  • Loading branch information
Alejandro Fernández Gómez authored Nov 30, 2022
1 parent 1c076ee commit 217a040
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ export const OverviewGrid = memo(() => {
) : (
<OverviewLoader />
)}
<span ref={intersectionRef}>
<div ref={intersectionRef}>
<EuiSpacer size="l" />
</span>
</div>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center">
{currentMonitors.length === monitors.length && (
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* 2.0.
*/

import { useEffect, useMemo, useState, useRef } from 'react';
import { isEqual } from 'lodash';
import { useMemo, useRef } from 'react';
import { useSelector } from 'react-redux';
import { MonitorOverviewItem } from '../../../../common/runtime_types';
import { selectOverviewState } from '../state/overview';
Expand All @@ -20,17 +19,19 @@ export function useMonitorsSortedByStatus() {
data: { monitors },
status,
} = useSelector(selectOverviewState);
const [monitorsSortedByStatus, setMonitorsSortedByStatus] = useState<
Record<string, MonitorOverviewItem[]>
>({ up: [], down: [], disabled: [] });

const downMonitors = useRef<Record<string, string[]> | null>(null);
const currentMonitors = useRef<MonitorOverviewItem[] | null>(monitors);
const locationNames = useLocationNames();

useEffect(() => {
const monitorsSortedByStatus = useMemo(() => {
if (!status) {
return;
return {
down: [],
up: [],
disabled: [],
};
}

const { downConfigs } = status;
const downMonitorMap: Record<string, string[]> = {};
downConfigs.forEach(({ location, configId }) => {
Expand All @@ -41,34 +42,30 @@ export function useMonitorsSortedByStatus() {
}
});

if (
!isEqual(downMonitorMap, downMonitors.current) ||
!isEqual(monitors, currentMonitors.current)
) {
const orderedDownMonitors: MonitorOverviewItem[] = [];
const orderedUpMonitors: MonitorOverviewItem[] = [];
const orderedDisabledMonitors: MonitorOverviewItem[] = [];
monitors.forEach((monitor) => {
const monitorLocation = locationNames[monitor.location.id];
if (!monitor.isEnabled) {
orderedDisabledMonitors.push(monitor);
} else if (
Object.keys(downMonitorMap).includes(monitor.id) &&
downMonitorMap[monitor.id].includes(monitorLocation)
) {
orderedDownMonitors.push(monitor);
} else {
orderedUpMonitors.push(monitor);
}
});
downMonitors.current = downMonitorMap;
currentMonitors.current = monitors;
setMonitorsSortedByStatus({
down: orderedDownMonitors,
up: orderedUpMonitors,
disabled: orderedDisabledMonitors,
});
}
const orderedDownMonitors: MonitorOverviewItem[] = [];
const orderedUpMonitors: MonitorOverviewItem[] = [];
const orderedDisabledMonitors: MonitorOverviewItem[] = [];

monitors.forEach((monitor) => {
const monitorLocation = locationNames[monitor.location.id];
if (!monitor.isEnabled) {
orderedDisabledMonitors.push(monitor);
} else if (
monitor.id in downMonitorMap &&
downMonitorMap[monitor.id].includes(monitorLocation)
) {
orderedDownMonitors.push(monitor);
} else {
orderedUpMonitors.push(monitor);
}
});
downMonitors.current = downMonitorMap;

return {
down: orderedDownMonitors,
up: orderedUpMonitors,
disabled: orderedDisabledMonitors,
};
}, [monitors, locationNames, downMonitors, status]);

return useMemo(() => {
Expand Down

0 comments on commit 217a040

Please sign in to comment.