Error and retry support for infinite scroll

web/packages/build/package.json

@@ -80,6 +80,7 @@
     "jest": "^27.3.1",
     "jest-styled-components": "^7.0.8",
     "jsdom": "^21.1.0",
+    "jsdom-testing-mocks": "^1.9.0",
     "msw": "^0.47.4",
     "optimist": "^0.6.1",
     "react": "^16.8.4",

web/packages/teleport/src/UnifiedResources/ResourceCard.tsx

@@ -323,7 +323,7 @@ function CopyButton({ name }: { name: string }) {
   );
 }
 
-function resourceName(resource: UnifiedResource) {
+export function resourceName(resource: UnifiedResource) {
   if (resource.kind === 'app' && resource.friendlyName) {
     return resource.friendlyName;
   }

web/packages/teleport/src/UnifiedResources/Resources.tsx

@@ -14,31 +14,41 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { useEffect, useRef, useState } from 'react';
+import React, { useEffect, useState } from 'react';
+
 import styled from 'styled-components';
-import { Box, Indicator, Flex, Text } from 'design';
+import {
+  Box,
+  Indicator,
+  Flex,
+  ButtonLink,
+  ButtonSecondary,
+  Text,
+} from 'design';
 import { Magnifier } from 'design/Icon';
 
+import { Danger } from 'design/Alert';
+
 import { TextIcon } from 'teleport/Discover/Shared';
 
 import {
   FeatureBox,
   FeatureHeader,
   FeatureHeaderTitle,
 } from 'teleport/components/Layout';
-import ErrorMessage from 'teleport/components/AgentErrorMessage';
 import Empty, { EmptyStateInfo } from 'teleport/components/Empty';
 import useTeleport from 'teleport/useTeleport';
 import cfg from 'teleport/config';
 import history from 'teleport/services/history/history';
 import localStorage from 'teleport/services/localStorage';
 import useStickyClusterId from 'teleport/useStickyClusterId';
 import AgentButtonAdd from 'teleport/components/AgentButtonAdd';
-import { useInfiniteScroll } from 'teleport/components/hooks/useInfiniteScroll';
 import { SearchResource } from 'teleport/Discover/SelectResource';
-import { useUrlFiltering } from 'teleport/components/hooks';
+import { useUrlFiltering, useInfiniteScroll } from 'teleport/components/hooks';
 
-import { ResourceCard, LoadingCard } from './ResourceCard';
+import { UnifiedResource } from 'teleport/services/agents';
+
+import { ResourceCard, LoadingCard, resourceName } from './ResourceCard';
 import SearchPanel from './SearchPanel';
 import { FilterPanel } from './FilterPanel';
 import './unifiedStyles.css';
@@ -58,34 +68,26 @@ export function Resources() {
   const canCreate = teleCtx.storeUser.getTokenAccess().create;
   const { clusterId } = useStickyClusterId();
 
-  const filtering = useUrlFiltering({
-    fieldName: 'name',
-    dir: 'ASC',
-  });
+  const { params, setParams, replaceHistory, pathname, setSort, onLabelClick } =
+    useUrlFiltering({
+      fieldName: 'name',
+      dir: 'ASC',
+    });
+
   const {
-    params,
-    search,
-    setParams,
-    replaceHistory,
-    pathname,
-    setSort,
-    onLabelClick,
-  } = filtering;
-
-  const { fetchInitial, fetchedData, attempt, fetchMore } = useInfiniteScroll({
+    setTrigger: setScrollDetector,
+    forceFetch,
+    resources,
+    attempt,
+  } = useInfiniteScroll({
     fetchFunc: teleCtx.resourceService.fetchUnifiedResources,
     clusterId,
+    filter: params,
     initialFetchSize: INITIAL_FETCH_SIZE,
     fetchMoreSize: FETCH_MORE_SIZE,
-    params,
   });
 
-  useEffect(() => {
-    fetchInitial();
-  }, [clusterId, search]);
-
-  const noResults =
-    attempt.status === 'success' && fetchedData.agents.length === 0;
+  const noResults = attempt.status === 'success' && resources.length === 0;
 
   const [isSearchEmpty, setIsSearchEmpty] = useState(true);
 
@@ -95,34 +97,14 @@ export function Resources() {
     setIsSearchEmpty(!params?.query && !params?.search);
   }, [params.query, params.search]);
 
-  const infiniteScrollDetector = useRef(null);
-
-  // Install the infinite scroll intersection observer.
-  //
-  // TODO(bl-nero): There's a known issue here. We need to have `fetchMore` in
-  // the list of hook dependencies, because using a stale `fetchMore` closure
-  // means we will fetch the same data over and over. However, as it's
-  // implemented now, every time `fetchMore` changes, we reinstall the observer.
-  // This is mitigated by `fetchMore` implementation, which doesn't spawn
-  // another request before the first one finishes, but it's still a potential
-  // for trouble in future. We need to decouple updating the `fetchMore` closure
-  // and installing the observer.
-  useEffect(() => {
-    if (infiniteScrollDetector.current) {
-      const observer = new IntersectionObserver(entries => {
-        if (entries[0]?.isIntersecting) {
-          fetchMore();
-        }
-      });
-      observer.observe(infiniteScrollDetector.current);
-      return () => observer.disconnect();
-    }
-  });
-
   if (!enabled) {
     history.replace(cfg.getNodesRoute(clusterId));
   }
 
+  const onRetryClicked = () => {
+    forceFetch();
+  };
+
   return (
     <FeatureBox
       className="ContainerContext"
@@ -132,6 +114,18 @@ export function Resources() {
         margin: auto;
       `}
     >
+      {attempt.status === 'failed' && (
+        <ErrorBox>
+          <ErrorBoxInternal>
+            <Danger>
+              {attempt.statusText}
+              <Box flex="0 0 auto" ml={2}>
+                <ButtonLink onClick={onRetryClicked}>Retry</ButtonLink>
+              </Box>
+            </Danger>
+          </ErrorBoxInternal>
+        </ErrorBox>
+      )}
       <FeatureHeader
         css={`
           border-bottom: none;
@@ -163,46 +157,45 @@ export function Resources() {
         pathname={pathname}
         replaceHistory={replaceHistory}
       />
-      {attempt.status === 'failed' && (
-        <ErrorMessage message={attempt.statusText} />
-      )}
       <ResourcesContainer className="ResourcesContainer" gap={2}>
-        {fetchedData.agents.map((agent, i) => (
-          <ResourceCard key={i} onLabelClick={onLabelClick} resource={agent} />
+        {resources.map(res => (
+          <ResourceCard
+            key={resourceKey(res)}
+            resource={res}
+            onLabelClick={onLabelClick}
+          />
         ))}
         {/* Using index as key here is ok because these elements never change order */}
         {attempt.status === 'processing' &&
           loadingCardArray.map((_, i) => <LoadingCard key={i} />)}
       </ResourcesContainer>
-      <div
-        ref={infiniteScrollDetector}
-        style={{
-          visibility: attempt.status === 'processing' ? 'visible' : 'hidden',
-        }}
-      >
-        {(attempt.status === 'processing' || fetchedData.startKey) && (
-          <Box
-            textAlign="center"
-            style={{ visible: attempt.status === 'processing' }}
-          >
-            <Indicator />
-          </Box>
+      <div ref={setScrollDetector} />
+      <ListFooter>
+        <IndicatorContainer status={attempt.status}>
+          <Indicator size={INDICATOR_SIZE} />
+        </IndicatorContainer>
+        {attempt.status === 'failed' && resources.length > 0 && (
+          <ButtonSecondary onClick={onRetryClicked}>Load more</ButtonSecondary>
         )}
-      </div>
-      {noResults && isSearchEmpty && (
-        <Empty
-          clusterId={clusterId}
-          canCreate={canCreate && !isLeafCluster}
-          emptyStateInfo={emptyStateInfo}
-        />
-      )}
-      {noResults && !isSearchEmpty && (
-        <NoResults query={params?.query || params?.search} />
-      )}
+        {noResults && isSearchEmpty && (
+          <Empty
+            clusterId={clusterId}
+            canCreate={canCreate && !isLeafCluster}
+            emptyStateInfo={emptyStateInfo}
+          />
+        )}
+        {noResults && !isSearchEmpty && (
+          <NoResults query={params?.query || params?.search} />
+        )}
+      </ListFooter>
     </FeatureBox>
   );
 }
 
+function resourceKey(resource: UnifiedResource) {
+  return `${resource.kind}/${resourceName(resource)}`;
+}
+
 function NoResults({ query }: { query: string }) {
   // Prevent `No resources were found for ""` flicker.
   if (query) {
@@ -234,6 +227,39 @@ const ResourcesContainer = styled(Flex)`
   grid-template-columns: repeat(auto-fill, minmax(400px, 1fr));
 `;
 
+const ErrorBox = styled(Box)`
+  position: sticky;
+  top: 0;
+  z-index: 1;
+`;
+
+const ErrorBoxInternal = styled(Box)`
+  position: absolute;
+  left: 0;
+  right: 0;
+  margin: ${props => props.theme.space[1]}px 10% 0 10%;
+`;
+
+const INDICATOR_SIZE = '48px';
+
+// It's important to make the footer at least as big as the loading indicator,
+// since in the typical case, we want to avoid UI "jumping" when loading the
+// final fragment finishes, and the final fragment is just one element in the
+// final row (i.e. the number of rows doesn't change). It's then important to
+// keep the same amount of whitespace below the resource list.
+const ListFooter = styled.div`
+  margin-top: ${props => props.theme.space[2]}px;
+  min-height: ${INDICATOR_SIZE};
+  text-align: center;
+`;
+
+// Line height is set to 0 to prevent the layout engine from adding extra pixels
+// to the element's height.
+const IndicatorContainer = styled(Box)`
+  display: ${props => (props.status === 'processing' ? 'block' : 'none')};
+  line-height: 0;
+`;
+
 const emptyStateInfo: EmptyStateInfo = {
   title: 'Add your first resource to Teleport',
   byline:

web/packages/teleport/src/components/hooks/index.ts

@@ -16,3 +16,4 @@
 
 export { useUrlFiltering } from './useUrlFiltering';
 export { useServerSidePagination } from './useServersidePagination';
+export { useInfiniteScroll } from './useInfiniteScroll';