From f1a8dc19e8b6a62fe6249ad137e3c23fda2d7f63 Mon Sep 17 00:00:00 2001
From: gintil <michael994664@gmail.com>
Date: Sat, 18 Jan 2025 15:24:42 -0500
Subject: [PATCH] Improve accessibility of comparisons tab content

---
 .../ArticlePlaceholderTable/index.tsx         |   2 +-
 .../AddComparisonSelect.tsx                   |  76 +++---
 .../ComparisonsTabSection/ComparisonTag.tsx   |  19 +-
 .../ComparisonsTabSection/index.tsx           | 257 +++++++++++-------
 .../client/src/mocks/data/userFeeds.ts        |   1 +
 .../backend-api/client/src/mocks/handlers.ts  |   2 +-
 6 files changed, 214 insertions(+), 143 deletions(-)

diff --git a/services/backend-api/client/src/features/feedConnections/components/ArticlePlaceholderTable/index.tsx b/services/backend-api/client/src/features/feedConnections/components/ArticlePlaceholderTable/index.tsx
index 3facdfbbf..367001a7e 100644
--- a/services/backend-api/client/src/features/feedConnections/components/ArticlePlaceholderTable/index.tsx
+++ b/services/backend-api/client/src/features/feedConnections/components/ArticlePlaceholderTable/index.tsx
@@ -75,7 +75,7 @@ const PlaceholderRow = ({
         </HStack>
       </Td>
       <Td whiteSpace="normal">
-        <Box maxHeight={150} overflow="auto">
+        <Box maxHeight={150} overflow="auto" tabIndex={0}>
           {value.split("\n").map((line, idx) => (
             // eslint-disable-next-line react/no-array-index-key
             <span key={idx}>
diff --git a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/AddComparisonSelect.tsx b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/AddComparisonSelect.tsx
index 07cc2692e..4fb9066db 100644
--- a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/AddComparisonSelect.tsx
+++ b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/AddComparisonSelect.tsx
@@ -1,5 +1,4 @@
-import { AddIcon } from "@chakra-ui/icons";
-import { Flex, IconButton, Select } from "@chakra-ui/react";
+import { Box, Button, Select, Stack, chakra } from "@chakra-ui/react";
 import { useState } from "react";
 
 interface Props {
@@ -7,9 +6,16 @@ interface Props {
   isDisabled?: boolean;
   isLoading?: boolean;
   onChange: (value: string) => Promise<void>;
+  formLabel: string;
 }
 
-export const AddComparisonSelect = ({ properties, isDisabled, isLoading, onChange }: Props) => {
+export const AddComparisonSelect = ({
+  properties,
+  isDisabled,
+  isLoading,
+  onChange,
+  formLabel,
+}: Props) => {
   const [value, setValue] = useState("");
   const [isAdding, setIsAdding] = useState(false);
 
@@ -21,35 +27,39 @@ export const AddComparisonSelect = ({ properties, isDisabled, isLoading, onChang
   };
 
   return (
-    <Flex>
-      <Select
-        size="sm"
-        borderTopLeftRadius="lg"
-        borderBottomLeftRadius="lg"
-        borderTopRightRadius={0}
-        borderBottomRightRadius={0}
-        isDisabled={isAdding || isDisabled}
-        minWidth={16}
-        borderRightStyle="none"
-        borderColor="gray.700"
-        placeholder="Select a property to add"
-        value={value}
-        onChange={(e) => setValue(e.target.value)}
-      >
-        {properties?.map((comparison) => (
-          <option value={comparison}>{comparison}</option>
-        ))}
-      </Select>
-      <IconButton
-        size="sm"
-        borderTopLeftRadius="0"
-        borderBottomLeftRadius={0}
-        icon={<AddIcon />}
-        isDisabled={isAdding || isDisabled || !value}
-        aria-label="Add comparison"
-        isLoading={isAdding || isLoading}
-        onClick={onClickAdd}
-      />
-    </Flex>
+    <Box as="form" onSubmit={() => onClickAdd()}>
+      <Stack>
+        {/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
+        <label>
+          <chakra.span paddingBottom="1" display="block" fontSize="sm" fontWeight="medium">
+            {formLabel}
+          </chakra.span>
+          <Select
+            size="sm"
+            isDisabled={isAdding || isDisabled}
+            minWidth={16}
+            placeholder="Select a property to add"
+            value={value}
+            onChange={(e) => setValue(e.target.value)}
+          >
+            {properties?.map((comparison) => (
+              <option value={comparison}>{comparison}</option>
+            ))}
+          </Select>
+        </label>
+        <Button
+          onClick={(e) => {
+            e.preventDefault();
+            onClickAdd();
+          }}
+          type="submit"
+          size="sm"
+          minW={32}
+          isLoading={isAdding || isLoading}
+        >
+          Add
+        </Button>
+      </Stack>
+    </Box>
   );
 };
diff --git a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/ComparisonTag.tsx b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/ComparisonTag.tsx
index 8580cbcdc..cd10e897d 100644
--- a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/ComparisonTag.tsx
+++ b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/ComparisonTag.tsx
@@ -1,13 +1,14 @@
-import { Box, Spinner, Tag, TagCloseButton, TagLabel, TagRightIcon } from "@chakra-ui/react";
+import { Spinner, Tag, TagCloseButton, TagLabel, TagRightIcon } from "@chakra-ui/react";
 import { useState } from "react";
 
 interface Props {
   colorScheme: string;
   title: string;
   onDelete: () => Promise<void>;
+  deleteButtonAriaLabel?: string;
 }
 
-export const ComparisonTag = ({ colorScheme, title, onDelete }: Props) => {
+export const ComparisonTag = ({ colorScheme, title, onDelete, deleteButtonAriaLabel }: Props) => {
   const [isDeleting, setIsDeleting] = useState(false);
 
   const onClickDelete = async () => {
@@ -17,12 +18,12 @@ export const ComparisonTag = ({ colorScheme, title, onDelete }: Props) => {
   };
 
   return (
-    <Box>
-      <Tag size="lg" colorScheme={colorScheme} margin={0}>
-        <TagLabel>{title}</TagLabel>
-        {isDeleting && <TagRightIcon as={Spinner} />}
-        {!isDeleting && <TagCloseButton aria-label="Delete" onClick={() => onClickDelete()} />}
-      </Tag>
-    </Box>
+    <Tag size="lg" colorScheme={colorScheme} margin={0} as="li">
+      <TagLabel>{title}</TagLabel>
+      {isDeleting && <TagRightIcon as={Spinner} />}
+      {!isDeleting && (
+        <TagCloseButton aria-label={deleteButtonAriaLabel} onClick={() => onClickDelete()} />
+      )}
+    </Tag>
   );
 };
diff --git a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/index.tsx b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/index.tsx
index 515544fb0..d791e343e 100644
--- a/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/index.tsx
+++ b/services/backend-api/client/src/features/feedConnections/components/ComparisonsTabSection/index.tsx
@@ -4,6 +4,7 @@ import {
   AlertIcon,
   AlertTitle,
   Button,
+  Divider,
   Flex,
   Heading,
   HStack,
@@ -177,7 +178,7 @@ export const ComparisonsTabSection = ({
   const hasAlert = !!(fetchErrorAlert || parseErrorAlert || noArticlesAlert);
 
   return (
-    <Stack spacing={16} marginBottom={16}>
+    <Stack spacing={12} marginBottom={16}>
       {error && (
         <Stack spacing={4}>
           <Alert status="error">
@@ -191,112 +192,170 @@ export const ComparisonsTabSection = ({
           </Alert>
         </Stack>
       )}
-      <Stack background="gray.700" padding={4} borderRadius="md">
-        <Flex justifyContent="space-between">
-          <Heading size="sm" as="h3">
-            Sample Article Properties
-          </Heading>
-          <ArticleSelectDialog
-            trigger={
-              <Button
-                leftIcon={<FiMousePointer />}
-                isLoading={!!selectedArticleId && userFeedArticlesFetchStatus === "fetching"}
-                isDisabled={userFeedArticlesFetchStatus === "fetching"}
-              >
-                {t("features.feedConnections.components.articlePlaceholderTable.selectArticle")}
-              </Button>
-            }
-            feedId={feedId}
-            onArticleSelected={onSelectedArticle}
-            onClickRandomArticle={onClickRandomFeedArticle}
-            articleFormatOptions={articleFormatOptions}
-          />
-        </Flex>
-        {fetchErrorAlert || parseErrorAlert || noArticlesAlert}
-        {userFeedArticlesStatus === "loading" && (
-          <Stack borderRadius="lg" background="gray.800" padding={4} alignItems="center">
-            <Spinner size="md" />
-            <Text>Loading article...</Text>
+      <Stack>
+        <Heading size="md" as="h2">
+          Comparisons
+        </Heading>
+        <Text>
+          If you are either not receiving articles or getting duplicate articles, you can add
+          additional properties that can be used to determine if an article should or should not get
+          delivered.
+        </Text>
+      </Stack>
+      <Stack spacing={8}>
+        <Stack spacing={4} border="1px solid" borderColor="gray.700" borderRadius="md" padding={4}>
+          <Stack>
+            <Heading size="sm" as="h3">
+              {t(
+                "features.feedConnections.components.comparisonsTabSection.passingComparisonsTitle"
+              )}
+            </Heading>
+            <Text>
+              {t(
+                "features.feedConnections.components.comparisonsTabSection.passingComparisonsDescription"
+              )}
+            </Text>
           </Stack>
-        )}
-        {!hasAlert && userFeedArticles?.result.articles.length && (
-          <Stack
-            maxHeight={400}
-            overflow="auto"
-            borderRadius="lg"
-            background="gray.800"
-            padding={4}
-          >
-            <ArticlePlaceholderTable
-              article={userFeedArticles?.result.articles[0]}
-              searchText=""
-              isFetching={userFeedArticlesFetchStatus === "fetching"}
-              withoutCopy
+          <Stack>
+            <Heading srOnly as="h4" id="passing-comparisons">
+              Current passing comparisons
+            </Heading>
+            {!passingComparisons?.length && (
+              <Text color="whiteAlpha.700">You currently have no passing comparisons created.</Text>
+            )}
+            {!!passingComparisons?.length && (
+              <HStack
+                flexWrap="wrap"
+                alignItems="center"
+                as="ul"
+                aria-labelledby="passing-comparisons"
+              >
+                {passingComparisons.map((comparison) => (
+                  <ComparisonTag
+                    title={comparison}
+                    colorScheme="cyan"
+                    onDelete={() => onRemovePassingComparison(comparison)}
+                    deleteButtonAriaLabel={`Delete passing comparison ${comparison}`}
+                  />
+                ))}
+              </HStack>
+            )}
+            <Divider my={2} />
+            <AddComparisonSelect
+              isDisabled={status !== "success"}
+              isLoading={status === "loading" || fetchStatus === "fetching"}
+              onChange={onAddPassingComparison}
+              properties={passingComaprisonsToAdd}
+              formLabel="Add a new passing comparison"
             />
           </Stack>
-        )}
-      </Stack>
-      <Stack spacing={4}>
-        <Stack>
-          <Heading size="md" as="h3">
-            {t("features.feedConnections.components.comparisonsTabSection.passingComparisonsTitle")}
-          </Heading>
-          <Text>
-            {t(
-              "features.feedConnections.components.comparisonsTabSection.passingComparisonsDescription"
-            )}
-          </Text>
+          {updateError && errorLocation === "passing" && (
+            <InlineErrorAlert title={t("common.errors.failedToSave")} description={updateError} />
+          )}
         </Stack>
-        <HStack flexWrap="wrap" alignItems="center">
-          {passingComparisons?.map((comparison) => (
-            <ComparisonTag
-              title={comparison}
-              colorScheme="cyan"
-              onDelete={() => onRemovePassingComparison(comparison)}
-            />
-          ))}
-          <AddComparisonSelect
-            isDisabled={status !== "success"}
-            isLoading={status === "loading" || fetchStatus === "fetching"}
-            onChange={onAddPassingComparison}
-            properties={passingComaprisonsToAdd}
-          />
-        </HStack>
-        {updateError && errorLocation === "passing" && (
-          <InlineErrorAlert title={t("common.errors.failedToSave")} description={updateError} />
-        )}
-      </Stack>
-      <Stack spacing={4}>
-        <Stack>
-          <Heading size="md" as="h3">
-            {t(
-              "features.feedConnections.components.comparisonsTabSection.blockingComparisonsTitle"
+        <Stack spacing={4} border="1px solid" borderColor="gray.700" borderRadius="md" padding={4}>
+          <Stack>
+            <Heading size="sm" as="h3">
+              {t(
+                "features.feedConnections.components.comparisonsTabSection.blockingComparisonsTitle"
+              )}
+            </Heading>
+            <Text>
+              {t(
+                "features.feedConnections.components.comparisonsTabSection.blockingComparisonsDescription"
+              )}
+            </Text>
+          </Stack>
+          <Stack>
+            <Heading srOnly as="h4" id="blocking-comparisons">
+              Current blocking comparisons
+            </Heading>
+            {!blockingComparisons?.length && (
+              <Text color="whiteAlpha.700">
+                You currently have no blocking comparisons created.
+              </Text>
             )}
-          </Heading>
-          <Text>
-            {t(
-              "features.feedConnections.components.comparisonsTabSection.blockingComparisonsDescription"
+            {!!blockingComparisons?.length && (
+              <HStack
+                flexWrap="wrap"
+                alignItems="center"
+                as="ul"
+                aria-labelledby="blocking-comparisons"
+              >
+                {blockingComparisons?.map((comparison) => (
+                  <ComparisonTag
+                    title={comparison}
+                    colorScheme="red"
+                    onDelete={() => onRemoveBlockingComparison(comparison)}
+                    deleteButtonAriaLabel={`Delete blocking comparison ${comparison}`}
+                  />
+                ))}
+              </HStack>
             )}
-          </Text>
+            <Divider my={2} />
+            <AddComparisonSelect
+              isDisabled={status !== "success"}
+              isLoading={status === "loading" || fetchStatus === "fetching"}
+              onChange={onAddBlockingComparison}
+              properties={blockingComparisonsToAdd}
+              formLabel="Add a new blocking comparison"
+            />
+          </Stack>
+          {updateError && errorLocation === "blocking" && (
+            <InlineErrorAlert title={t("common.errors.failedToSave")} description={updateError} />
+          )}
         </Stack>
-        <HStack flexWrap="wrap" alignItems="center">
-          {blockingComparisons?.map((comparison) => (
-            <ComparisonTag
-              title={comparison}
-              colorScheme="red"
-              onDelete={() => onRemoveBlockingComparison(comparison)}
+        <Stack background="gray.700" padding={4} borderRadius="md" as="aside">
+          <Flex justifyContent="space-between" flexWrap="wrap">
+            <Stack mb={1}>
+              <Heading size="sm" as="h3">
+                Preview Sample Article Properties
+              </Heading>
+              <Text>
+                Preview an article to see the properties that can be used for comparisons.
+              </Text>
+            </Stack>
+            <ArticleSelectDialog
+              trigger={
+                <Button
+                  leftIcon={<FiMousePointer />}
+                  isLoading={!!selectedArticleId && userFeedArticlesFetchStatus === "fetching"}
+                  isDisabled={userFeedArticlesFetchStatus === "fetching"}
+                  size="sm"
+                >
+                  Select article to preview
+                </Button>
+              }
+              feedId={feedId}
+              onArticleSelected={onSelectedArticle}
+              onClickRandomArticle={onClickRandomFeedArticle}
+              articleFormatOptions={articleFormatOptions}
             />
-          ))}
-          <AddComparisonSelect
-            isDisabled={status !== "success"}
-            isLoading={status === "loading" || fetchStatus === "fetching"}
-            onChange={onAddBlockingComparison}
-            properties={blockingComparisonsToAdd}
-          />
-        </HStack>
-        {updateError && errorLocation === "blocking" && (
-          <InlineErrorAlert title={t("common.errors.failedToSave")} description={updateError} />
-        )}
+          </Flex>
+          {fetchErrorAlert || parseErrorAlert || noArticlesAlert}
+          {userFeedArticlesStatus === "loading" && (
+            <Stack borderRadius="lg" background="gray.800" padding={4} alignItems="center">
+              <Spinner size="md" />
+              <Text>Loading article...</Text>
+            </Stack>
+          )}
+          {!hasAlert && userFeedArticles?.result.articles.length && (
+            <Stack
+              maxHeight={400}
+              overflow="auto"
+              borderRadius="lg"
+              background="gray.800"
+              padding={4}
+            >
+              <ArticlePlaceholderTable
+                article={userFeedArticles?.result.articles[0]}
+                searchText=""
+                isFetching={userFeedArticlesFetchStatus === "fetching"}
+                withoutCopy
+              />
+            </Stack>
+          )}
+        </Stack>
       </Stack>
     </Stack>
   );
diff --git a/services/backend-api/client/src/mocks/data/userFeeds.ts b/services/backend-api/client/src/mocks/data/userFeeds.ts
index 5e1d0a43a..08b0c3a0b 100644
--- a/services/backend-api/client/src/mocks/data/userFeeds.ts
+++ b/services/backend-api/client/src/mocks/data/userFeeds.ts
@@ -320,6 +320,7 @@ const mockUserFeeds: UserFeed[] = [
     updatedAt: new Date().toISOString(),
     refreshRateOptions: [],
     healthStatus: UserFeedHealthStatus.Failed,
+    passingComparisons: ["title", "description"],
     connections: [],
     disabledCode: UserFeedDisabledCode.Manual,
     refreshRateSeconds: 60,
diff --git a/services/backend-api/client/src/mocks/handlers.ts b/services/backend-api/client/src/mocks/handlers.ts
index 01132c236..99c269d51 100644
--- a/services/backend-api/client/src/mocks/handlers.ts
+++ b/services/backend-api/client/src/mocks/handlers.ts
@@ -643,7 +643,7 @@ const handlers = [
 
     return res(
       ctx.delay(500),
-      ctx.status(404),
+      ctx.status(200),
       ctx.json<UpdateUserFeedOutput>({
         result: matchingUserFeed,
       })