🐛 Fix target card behavior and rendering

Resolves: #2022 Resolves: #2010 Resolves: #1981 Resolves: https://issues.redhat.com/browse/MTA-3094 Target card behavior and rendering on the target selection step of the analysis wizard and on the custom migrations target page have been updated: - All of the card contents render inside the card itself (#1981 / MTA-3094) - When changing the specific label selection for a target, the card selection will not changes (#2010) - The target label select now sorts the labels in numeric natural sort descending order. `OpenJDK 21` will be initially selected and appear above `OpenJDK 17` in the select list. (#2022) - Layouts used on the card have been refactored to use base layouts instead of the `EmptyState` component - The target label select box has been updated to the current `SimpleSelectBasic` component for current PF5 alignment - When card selection is enabled (on the target selection step of the analysis wizard), the selection checkbox is always displayed and the normal `Card` hover styles have been disabled. Some refactoring on the `SetTargets` component used by the analysis wizard have been made to simplify rendering code. Signed-off-by: Scott J Dickerson <[email protected]>
konveyor · Jul 24, 2024 · ad33e89 · ad33e89
1 parent acc23ac
commit ad33e89
Show file tree

Hide file tree

Showing 3 changed files with 155 additions and 132 deletions.
diff --git a/client/src/app/components/target-card/target-card.css b/client/src/app/components/target-card/target-card.css
@@ -2,10 +2,26 @@
   background: none;
 }
 
-.select-card__component__empty-state {
-  padding: 0 !important;
+/* do NOT change background-color or box-shadow on hover with the TargetCard is selectable */
+.target-card.pf-m-selectable {
+  --pf-v5-c-card--m-selectable--hover--BackgroundColor: var(
+    --pf-v5-c-card--BackgroundColor
+  );
+  --pf-v5-c-card--m-selectable--hover--BoxShadow: var(
+    --pf-v5-c-card--BoxShadow
+  );
 }
 
+/*
+  A way to force the select box to always have a white background,
+  even when the card is selected
+*/
+/*
+.target-label-choice-container {
+  background-color: var(--pf-v5-global--BackgroundColor--100);
+}
+*/
+
 .grabbable {
   cursor: move; /* fallback if grab cursor is unsupported */
   cursor: grab;

diff --git a/client/src/app/components/target-card/target-card.tsx b/client/src/app/components/target-card/target-card.tsx
@@ -1,14 +1,10 @@
-import "./target-card.css";
 import * as React from "react";
 import {
-  EmptyState,
   EmptyStateIcon,
   Title,
-  EmptyStateVariant,
   Card,
   CardBody,
   DropdownItem,
-  Text,
   Flex,
   FlexItem,
   Button,
@@ -18,26 +14,26 @@ import {
   PanelMain,
   PanelMainBody,
   Panel,
+  Stack,
+  StackItem,
+  Bullseye,
 } from "@patternfly/react-core";
-import {
-  Select,
-  SelectOption,
-  SelectVariant,
-  SelectOptionObject,
-} from "@patternfly/react-core/deprecated";
 import { GripVerticalIcon, InfoCircleIcon } from "@patternfly/react-icons";
 import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
 import { useTranslation } from "react-i18next";
 
-import { KebabDropdown } from "../KebabDropdown";
 import DefaultImage from "@app/images/Icon-Red_Hat-Virtual_server_stack-A-Black-RGB.svg";
 import { Target, TargetLabel } from "@app/api/models";
+import { KebabDropdown } from "../KebabDropdown";
 import useFetchImageDataUrl from "./hooks/useFetchImageDataUrl";
+import { SimpleSelectBasic } from "../SimpleSelectBasic";
+
+import "./target-card.css";
+import { localeNumericCompare } from "@app/utils/utils";
 
 export interface TargetCardProps {
   item: Target;
   cardSelected?: boolean;
-  isEditable?: boolean;
   onCardClick?: (
     isSelecting: boolean,
     targetLabelName: string,
@@ -51,14 +47,16 @@ export interface TargetCardProps {
   onDelete?: () => void;
 }
 
-// Force display dropdown box even though there only one option available.
-// This is a business rule to guarantee that option is always present.
+/**
+ * Force display dropdown box even though there only one option available.
+ * This is a business rule to guarantee that option is always present.
+ */
 const forceSelect = ["Azure"];
 
 export const TargetCard: React.FC<TargetCardProps> = ({
   item: target,
-  readOnly,
-  cardSelected,
+  readOnly = false,
+  cardSelected = false,
   formLabels,
   onCardClick,
   onSelectedCardTargetChange,
@@ -67,61 +65,82 @@ export const TargetCard: React.FC<TargetCardProps> = ({
   onDelete,
 }) => {
   const { t } = useTranslation();
-  const [isCardSelected, setCardSelected] = React.useState(cardSelected);
   const imageDataUrl = useFetchImageDataUrl(target);
 
-  const prevSelectedLabel =
-    formLabels?.find((formLabel) => {
-      const labelNames = target?.labels?.map((label) => label.name);
-      return labelNames?.includes(formLabel.name);
-    })?.name || "";
-
-  const [isLabelSelectOpen, setLabelSelectOpen] = React.useState(false);
+  const targetLabels = (target?.labels ?? []).sort((a, b) =>
+    localeNumericCompare(b.label, a.label)
+  );
 
   const [selectedLabelName, setSelectedLabelName] = React.useState<string>(
-    prevSelectedLabel ||
-      target?.labels?.[0]?.name ||
-      `${target?.name || "target"}-Empty`
+    () => {
+      const prevSelectedLabel =
+        formLabels?.find((formLabel) => {
+          const labelNames = targetLabels.map((label) => label.name);
+          return labelNames?.includes(formLabel.name);
+        })?.name || "";
+
+      return (
+        prevSelectedLabel ||
+        targetLabels[0]?.name ||
+        `${target?.name || "target"}-Empty`
+      );
+    }
   );
 
   const handleCardClick = (event: React.MouseEvent) => {
     const eventTarget = event.target as HTMLElement;
+    event.preventDefault();
 
-    if (eventTarget.tagName === "BUTTON" || eventTarget.tagName === "LABEL") {
-      event.preventDefault();
+    // Let the label choice select box do its own click handling
+    if (eventTarget.closest(".target-label-choice-container")) {
+      return;
     }
 
-    setCardSelected(!isCardSelected);
     if (onCardClick && selectedLabelName) {
-      onCardClick(!isCardSelected, selectedLabelName, target);
+      onCardClick(!cardSelected, selectedLabelName, target);
     }
   };
 
-  const handleLabelSelection = (
-    event: React.MouseEvent | React.ChangeEvent,
-    selection: string | SelectOptionObject
-  ) => {
-    event.stopPropagation();
-    setLabelSelectOpen(false);
+  const handleLabelSelection = (selection: string) => {
     setSelectedLabelName(selection as string);
-    if (isCardSelected && onSelectedCardTargetChange) {
+    if (cardSelected && onSelectedCardTargetChange) {
       onSelectedCardTargetChange(selection as string);
     }
   };
+
+  const TargetLogo = () => (
+    <img
+      src={imageDataUrl || DefaultImage}
+      alt="Card logo"
+      style={{ height: 80, pointerEvents: "none" }}
+      onError={(e) => {
+        e.currentTarget.src = DefaultImage;
+      }}
+    />
+  );
+
+  const labelChoices =
+    target.choice || forceSelect.includes(target.name) ? targetLabels : [];
+
   return (
     <Card
+      className="target-card"
       id={`target-card-${target.name.replace(/\s/g, "-")}`}
+      data-target-name={target.name}
+      data-target-id={target.id}
       onClick={handleCardClick}
-      isSelectable={!!cardSelected}
-      isSelected={isCardSelected}
-      className="pf-v5-l-stack pf-v5-l-stack__item pf-m-fill"
+      isSelectable={readOnly}
+      isSelected={cardSelected}
+      isFullHeight
+      isCompact
+      isFlat
     >
       <CardHeader
-        checked={isCardSelected}
+        checked={cardSelected}
         selectableActions={{
           selectableActionId: "target-name-" + target.name,
           selectableActionAriaLabelledby: `${target.name}-selectable-action-label`,
-          isChecked: isCardSelected,
+          isChecked: cardSelected,
         }}
       >
         <Label
@@ -145,7 +164,7 @@ export const TargetCard: React.FC<TargetCardProps> = ({
                 {...handleProps?.listeners}
                 {...handleProps?.attributes}
               >
-                <GripVerticalIcon></GripVerticalIcon>
+                <GripVerticalIcon />
               </Button>
             )}
           </FlexItem>
@@ -168,58 +187,52 @@ export const TargetCard: React.FC<TargetCardProps> = ({
             )}
           </FlexItem>
         </Flex>
-        <EmptyState
-          variant={EmptyStateVariant.sm}
-          className="select-card__component__empty-state"
-        >
-          <EmptyStateIcon
-            icon={() => (
-              <img
-                src={imageDataUrl || DefaultImage}
-                alt="Card logo"
-                style={{ height: 80, pointerEvents: "none" }}
-                onError={(e) => {
-                  e.currentTarget.src = DefaultImage;
+
+        <Stack hasGutter>
+          <StackItem>
+            <Bullseye>
+              <EmptyStateIcon color="black" icon={TargetLogo} />
+            </Bullseye>
+          </StackItem>
+          <StackItem>
+            <Bullseye>
+              <Title headingLevel="h4" size="md">
+                {target.name}
+              </Title>
+            </Bullseye>
+          </StackItem>
+
+          {/* Target label choice */}
+          {labelChoices.length === 0 ? null : (
+            <StackItem className="target-label-choice-container">
+              <SimpleSelectBasic
+                selectId={`${target.name}-label-menu`}
+                toggleId={`${target.name}-toggle`}
+                toggleAriaLabel="Select label dropdown target"
+                aria-label="Select Label"
+                value={selectedLabelName}
+                options={labelChoices.map((label) => ({
+                  children: label.name,
+                  value: label.name,
+                }))}
+                onChange={(option) => {
+                  handleLabelSelection(option);
                 }}
               />
-            )}
-          />
-          <Title headingLevel="h4" size="md">
-            {target.name}
-          </Title>
-          {target.choice &&
-          ((!!target?.labels?.length && target?.labels?.length > 1) ||
-            forceSelect.includes(target.name)) ? (
-            <Select
-              className={spacing.mtSm}
-              toggleId={`${target.name}-toggle`}
-              variant={SelectVariant.single}
-              aria-label="Select Label"
-              onToggle={(_, isExpanded) => setLabelSelectOpen(isExpanded)}
-              onSelect={handleLabelSelection}
-              selections={selectedLabelName}
-              isOpen={isLabelSelectOpen}
-              width={250}
-            >
-              {target?.labels?.map((label) => (
-                <SelectOption key={label.name} value={label.name}>
-                  {label.name ? label.name : "Empty"}
-                </SelectOption>
-              ))}
-            </Select>
-          ) : null}
-          {target.description ? (
-            <Panel isScrollable className="panel-style">
-              <PanelMain maxHeight={target.choice ? "9em" : "12em"}>
-                <PanelMainBody>
-                  <Text className={`${spacing.pMd} pf-v5-u-text-align-left`}>
-                    {target.description}
-                  </Text>
-                </PanelMainBody>
-              </PanelMain>
-            </Panel>
-          ) : null}
-        </EmptyState>
+            </StackItem>
+          )}
+
+          {/* Target description */}
+          <StackItem isFilled>
+            {target.description ? (
+              <Panel isScrollable className="panel-style">
+                <PanelMain maxHeight={target.choice ? "9em" : "12em"}>
+                  <PanelMainBody>{target.description}</PanelMainBody>
+                </PanelMain>
+              </Panel>
+            ) : null}
+          </StackItem>
+        </Stack>
       </CardBody>
     </Card>
   );