Skip to content

Commit

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

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Synthetics UI] Fix infinite scroll in overview page
(#146570)](#146570)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alejandro Fernández
Gómez","email":"alejandro.fernandez@elastic.co"},"sourceCommit":{"committedDate":"2022-11-30T08:23:34Z","message":"[Synthetics
UI] Fix infinite scroll in overview page (#146570)\n\n##
Summary\r\n\r\nCloses #145378.\r\n\r\nThe overview page has a
[`<EuiSpacer />` at the bottom of
the\r\ngrid](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L135-L137)\r\nto
track the user's scroll position. This is done [with
an\r\n`IntersectionObserver` (wrapped in a hook) and
a\r\n`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).\r\n\r\nSometimes
the `ref` got destroyed, and with it the\r\n`IntersectionObserver`
instance. This causes a race condition [in the\r\ncode that checks for
the\r\nintersection](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L78-L88)\r\nto
determine if the next page should be loaded or not.\r\n\r\n<img
width=\"1278\" alt=\"Screenshot 2022-11-29 at 16 35
42\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204574461-5056391c-e96d-4b38-9c9e-4976e0d14a40.png\">\r\n\r\n\r\nThe
reason why the `ref` got destroyed was [this
early\r\nreturn](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L98-L100).\r\nWith
the code in `main` there is a brief moment in which the
condition\r\nholds `true`, right after the monitors are loaded. In that
brief instant\r\nthe `status` is present but `monitorsSortedByStatus` is
empty.\r\n\r\n<img width=\"1084\" alt=\"Screenshot 2022-11-29 at 16 44
35\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204575679-de4bc6bf-122b-4c6d-ae75-9c96a7c5fb85.png\">\r\n\r\nWhen
this happens the `ref` is destroyed, because the underlying
element\r\nused to track the intersection is destroyed due of the early
return.\r\n\r\nThis was caused because `monitorsSortedByStatus` was
updated\r\nasynchronously
[with\r\n`useEffect`](https://github.com/elastic/kibana/blob/2f3313371bc6d992a99accef1289dd035779d3e6/x-pack/plugins/synthetics/public/apps/synthetics/hooks/use_monitors_sorted_by_status.tsx#L30-L72)\r\nas
part of the component lifecycle.\r\n\r\nThis PR uses `useMemo` instead
of `useEffect`, to update the\r\n`monitorsSortedByStatus` at the same
time that `status` is present. This\r\nprevents the early return from
ever firing in the normal load cycle and\r\nkeeps the `ref`. Since the
`ref` never gets destroyed there's no race\r\ncondition
anymore.","sha":"217a04078d4f8906feaf555da81aff8ce3d8ffad","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:uptime","release_note:skip","v8.6.0","v8.7.0"],"number":146570,"url":"https://github.com/elastic/kibana/pull/146570","mergeCommit":{"message":"[Synthetics
UI] Fix infinite scroll in overview page (#146570)\n\n##
Summary\r\n\r\nCloses #145378.\r\n\r\nThe overview page has a
[`<EuiSpacer />` at the bottom of
the\r\ngrid](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L135-L137)\r\nto
track the user's scroll position. This is done [with
an\r\n`IntersectionObserver` (wrapped in a hook) and
a\r\n`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).\r\n\r\nSometimes
the `ref` got destroyed, and with it the\r\n`IntersectionObserver`
instance. This causes a race condition [in the\r\ncode that checks for
the\r\nintersection](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L78-L88)\r\nto
determine if the next page should be loaded or not.\r\n\r\n<img
width=\"1278\" alt=\"Screenshot 2022-11-29 at 16 35
42\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204574461-5056391c-e96d-4b38-9c9e-4976e0d14a40.png\">\r\n\r\n\r\nThe
reason why the `ref` got destroyed was [this
early\r\nreturn](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L98-L100).\r\nWith
the code in `main` there is a brief moment in which the
condition\r\nholds `true`, right after the monitors are loaded. In that
brief instant\r\nthe `status` is present but `monitorsSortedByStatus` is
empty.\r\n\r\n<img width=\"1084\" alt=\"Screenshot 2022-11-29 at 16 44
35\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204575679-de4bc6bf-122b-4c6d-ae75-9c96a7c5fb85.png\">\r\n\r\nWhen
this happens the `ref` is destroyed, because the underlying
element\r\nused to track the intersection is destroyed due of the early
return.\r\n\r\nThis was caused because `monitorsSortedByStatus` was
updated\r\nasynchronously
[with\r\n`useEffect`](https://github.com/elastic/kibana/blob/2f3313371bc6d992a99accef1289dd035779d3e6/x-pack/plugins/synthetics/public/apps/synthetics/hooks/use_monitors_sorted_by_status.tsx#L30-L72)\r\nas
part of the component lifecycle.\r\n\r\nThis PR uses `useMemo` instead
of `useEffect`, to update the\r\n`monitorsSortedByStatus` at the same
time that `status` is present. This\r\nprevents the early return from
ever firing in the normal load cycle and\r\nkeeps the `ref`. Since the
`ref` never gets destroyed there's no race\r\ncondition
anymore.","sha":"217a04078d4f8906feaf555da81aff8ce3d8ffad"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146570","number":146570,"mergeCommit":{"message":"[Synthetics
UI] Fix infinite scroll in overview page (#146570)\n\n##
Summary\r\n\r\nCloses #145378.\r\n\r\nThe overview page has a
[`<EuiSpacer />` at the bottom of
the\r\ngrid](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L135-L137)\r\nto
track the user's scroll position. This is done [with
an\r\n`IntersectionObserver` (wrapped in a hook) and
a\r\n`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).\r\n\r\nSometimes
the `ref` got destroyed, and with it the\r\n`IntersectionObserver`
instance. This causes a race condition [in the\r\ncode that checks for
the\r\nintersection](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L78-L88)\r\nto
determine if the next page should be loaded or not.\r\n\r\n<img
width=\"1278\" alt=\"Screenshot 2022-11-29 at 16 35
42\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204574461-5056391c-e96d-4b38-9c9e-4976e0d14a40.png\">\r\n\r\n\r\nThe
reason why the `ref` got destroyed was [this
early\r\nreturn](https://github.com/elastic/kibana/blob/e8d77b3f0f46ac62e9220ffe28eb455880854906/x-pack/plugins/synthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx#L98-L100).\r\nWith
the code in `main` there is a brief moment in which the
condition\r\nholds `true`, right after the monitors are loaded. In that
brief instant\r\nthe `status` is present but `monitorsSortedByStatus` is
empty.\r\n\r\n<img width=\"1084\" alt=\"Screenshot 2022-11-29 at 16 44
35\"\r\nsrc=\"https://user-images.githubusercontent.com/57448/204575679-de4bc6bf-122b-4c6d-ae75-9c96a7c5fb85.png\">\r\n\r\nWhen
this happens the `ref` is destroyed, because the underlying
element\r\nused to track the intersection is destroyed due of the early
return.\r\n\r\nThis was caused because `monitorsSortedByStatus` was
updated\r\nasynchronously
[with\r\n`useEffect`](https://github.com/elastic/kibana/blob/2f3313371bc6d992a99accef1289dd035779d3e6/x-pack/plugins/synthetics/public/apps/synthetics/hooks/use_monitors_sorted_by_status.tsx#L30-L72)\r\nas
part of the component lifecycle.\r\n\r\nThis PR uses `useMemo` instead
of `useEffect`, to update the\r\n`monitorsSortedByStatus` at the same
time that `status` is present. This\r\nprevents the early return from
ever firing in the normal load cycle and\r\nkeeps the `ref`. Since the
`ref` never gets destroyed there's no race\r\ncondition
anymore.","sha":"217a04078d4f8906feaf555da81aff8ce3d8ffad"}}]}]
BACKPORT-->

Co-authored-by: Alejandro Fernández Gómez <alejandro.fernandez@elastic.co>
  • Loading branch information
kibanamachine and Alejandro Fernández Gómez authored Nov 30, 2022
1 parent b1406b8 commit 5c2f7e9
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 5c2f7e9

Please sign in to comment.