fix(Card): updated v5 logic to prevent unclickable cards (#10202)

* fix(Card): updated v5 logic to prevent unclickable cards * Updated API to have isSelected control selectable card state * Updated cypress test * Updated cypress test * Updated cypress test
patternfly · Mar 28, 2024 · 779e288 · 779e288
1 parent 5fc4f38
commit 779e288
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 53 deletions.
diff --git a/packages/react-core/src/components/Card/Card.tsx b/packages/react-core/src/components/Card/Card.tsx
@@ -14,15 +14,21 @@ export interface CardProps extends React.HTMLProps<HTMLElement>, OUIAProps {
   component?: keyof JSX.IntrinsicElements;
   /** Modifies the card to include compact styling. Should not be used with isLarge. */
   isCompact?: boolean;
-  /** Modifies the card to include selectable styling */
+  /** Flag indicating that the card is selectable. */
   isSelectable?: boolean;
   /** @deprecated Specifies the card is selectable, and applies raised styling on hover and select */
   isSelectableRaised?: boolean;
-  /** Modifies the card to include selected styling */
-  isSelected?: boolean;
-  /** Modifies the card to include clickable styling */
+  /** Flag indicating that the card is clickable and contains some action that triggers on click. */
   isClickable?: boolean;
-  /** Modifies a clickable or selectable card to have disabled styling. */
+  /** Flag indicating whether a card that is either selectable only or both clickable and selectable is
+   * currently selected and has selected styling.
+   */
+  isSelected?: boolean;
+  /** Flag indicating whether a card that is either only clickable or that is both clickable and selectable
+   * is currently clicked and has clicked styling.
+   */
+  isClicked?: boolean;
+  /** Flag indicating that a clickable or selectable card is disabled. */
   isDisabled?: boolean;
   /** @deprecated Modifies a raised selectable card to have disabled styling */
   isDisabledRaised?: boolean;
@@ -56,6 +62,8 @@ interface CardContextProps {
   isExpanded: boolean;
   isClickable: boolean;
   isSelectable: boolean;
+  isSelected: boolean;
+  isClicked: boolean;
   isDisabled: boolean;
   // TODO: Remove hasSelectableInput when deprecated prop is removed
   hasSelectableInput: boolean;
@@ -72,6 +80,8 @@ export const CardContext = React.createContext<Partial<CardContextProps>>({
   isExpanded: false,
   isClickable: false,
   isSelectable: false,
+  isSelected: false,
+  isClicked: false,
   isDisabled: false
 });
 
@@ -86,6 +96,7 @@ export const Card: React.FunctionComponent<CardProps> = ({
   isDisabled = false,
   isSelectableRaised = false,
   isSelected = false,
+  isClicked = false,
   isDisabledRaised = false,
   isFlat = false,
   isExpanded = false,
@@ -119,15 +130,19 @@ export const Card: React.FunctionComponent<CardProps> = ({
       return css(styles.modifiers.selectableRaised, isSelected && styles.modifiers.selectedRaised);
     }
     if (isSelectable && isClickable) {
-      return css(styles.modifiers.selectable, styles.modifiers.clickable, isSelected && styles.modifiers.current);
+      return css(
+        styles.modifiers.selectable,
+        styles.modifiers.clickable,
+        (isSelected || isClicked) && styles.modifiers.current
+      );
     }
 
     if (isSelectable) {
       return css(styles.modifiers.selectable, isSelected && styles.modifiers.selected);
     }
 
     if (isClickable) {
-      return css(styles.modifiers.clickable, isSelected && styles.modifiers.selected);
+      return css(styles.modifiers.clickable, isClicked && styles.modifiers.current);
     }
 
     return '';
@@ -162,6 +177,8 @@ export const Card: React.FunctionComponent<CardProps> = ({
         isExpanded,
         isClickable,
         isSelectable,
+        isSelected,
+        isClicked,
         isDisabled,
         // TODO: Remove hasSelectableInput when deprecated prop is removed
         hasSelectableInput

diff --git a/packages/react-core/src/components/Card/CardHeader.tsx b/packages/react-core/src/components/Card/CardHeader.tsx
@@ -44,7 +44,9 @@ export interface CardHeaderSelectableActionsObject {
   isExternalLink?: boolean;
   /** Name for the input element of a clickable or selectable card. */
   name?: string;
-  /** Flag indicating whether the selectable card input is checked */
+  /** @deprecated Flag indicating whether the selectable card input is checked. We recommend using
+   * the isSelected prop on the card component instead.
+   */
   isChecked?: boolean;
 }
 
@@ -80,7 +82,7 @@ export const CardHeader: React.FunctionComponent<CardHeaderProps> = ({
 }: CardHeaderProps) => (
   <CardContext.Consumer>
     {/* TODO: Remove hasSelectableInput when deprecated props are removed */}
-    {({ cardId, isClickable, isSelectable, isDisabled: isCardDisabled, hasSelectableInput }) => {
+    {({ cardId, isClickable, isSelectable, isSelected, isClicked, isDisabled: isCardDisabled, hasSelectableInput }) => {
       const cardHeaderToggle = (
         <div className={css(styles.cardHeaderToggle)}>
           <Button
@@ -112,12 +114,10 @@ export const CardHeader: React.FunctionComponent<CardHeaderProps> = ({
       }
 
       const handleActionClick = (event: React.FormEvent<HTMLInputElement> | React.MouseEvent) => {
-        if (isClickable) {
-          if (selectableActions?.onClickAction) {
-            selectableActions.onClickAction(event);
-          } else if (selectableActions?.to) {
-            window.open(selectableActions.to, selectableActions.isExternalLink ? '_blank' : '_self');
-          }
+        if (selectableActions?.onClickAction) {
+          selectableActions.onClickAction(event);
+        } else if (selectableActions?.to) {
+          window.open(selectableActions.to, selectableActions.isExternalLink ? '_blank' : '_self');
         }
       };
 
@@ -132,12 +132,13 @@ export const CardHeader: React.FunctionComponent<CardHeaderProps> = ({
           name: selectableActions.name,
           isDisabled: isCardDisabled
         };
+        const isSelectableInputChecked = selectableActions.isChecked ?? isSelected;
 
         if (isClickable && !isSelectable) {
-          return { ...baseProps, onClick: handleActionClick };
+          return { ...baseProps, onClick: handleActionClick, isChecked: isClicked };
         }
         if (isSelectable) {
-          return { ...baseProps, onChange: selectableActions.onChange, isChecked: selectableActions.isChecked };
+          return { ...baseProps, onChange: selectableActions.onChange, isChecked: isSelectableInputChecked };
         }
 
         return baseProps;

diff --git a/packages/react-core/src/components/Card/examples/Card.md b/packages/react-core/src/components/Card/examples/Card.md
@@ -109,37 +109,39 @@ A common use case of this is to set all but one body section to `isFilled={false
 
 ### Selectable
 
-A card can be selected by clicking anywhere within the card.
+A selectable card can be selected by clicking anywhere within the card.
 
-You must avoid rendering any other interactive content within the `<Card>` when it is meant to be selectable only. Refer to our [clickable and selectable example](#clickable-and-selectable-cards) if you need a card that is both selectable and has other interactive content.
+You must avoid rendering any other interactive content within the `<Card>` when it is meant to be selectable only. Refer to our [actionable and selectable example](#clickable-and-selectable-cards) if you need a card that is both selectable and has other interactive, actionable content.
 
 ```ts file='./CardSelectable.tsx'
 
 ```
 
 ### Single selectable
 
-When a group of single selectable cards are related, you should pass the same `name` property to each card's `selectableActions` property.
+When a group of single selectable cards are related, you must pass the same `name` property to each card's `selectableActions` property.
 
 ```ts file='./CardSingleSelectable.tsx'
 
 ```
 
-### Clickable
+### Actionable
 
-A card can perform an action or navigate to a link by clicking anywhere within the card. You can also pass in the `isExternalLink` property to `selectableActions` if you want a clickable card's link to open in a new tab or window.
+An actionable card can perform an action or navigate to a link by clicking anywhere within the card. To open a link in a new tab or window, pass the `isExternalLink` property to `selectableActions`.
 
-When a card is meant to be clickable only, you must avoid rendering any other interactive content within the `<Card>`, similar to selectable cards.
+You can pass the `isClicked` property to `<Card>` to convey that a card is the currently clicked one, such as when clicking a card would open a [primary-detail view](/patterns/primary-detail). This must not be used simply for "selection" of a card, and you should instead use our [selectable card](#selectable) or [single selectable card](#single-selectable).
+
+When a card is meant to be actionable only, you must avoid rendering any other interactive content within the `<Card>`, similar to selectable cards.
 
 ```ts file='./CardClickable.tsx'
 
 ```
 
-### Clickable and selectable
+### Actionable and selectable
 
-A card can be selectable and have additional interactive content by passing both the `isClickable` and `isSelectable` properties to `<Card>`. The following example shows how the "clickable" functionality can be rendered anywhere within a selectable card.
+A card can be selectable and have additional interactive content by passing both the `isClickable` and `isSelectable` properties to `<Card>`. The following example shows how the actionable functionality can be rendered anywhere within a selectable card.
 
-When passing interactive content to a clickable and selectable card that is disabled, you should also ensure the interactive content is disabled as well, if applicable.
+When passing interactive content to an actionable and selectable card that is disabled, you should also ensure the interactive content is disabled as well, if applicable.
 
 ```ts file='./CardClickableSelectable.tsx'
 

diff --git a/packages/react-core/src/components/Card/examples/CardClickableSelectable.tsx b/packages/react-core/src/components/Card/examples/CardClickableSelectable.tsx
@@ -5,14 +5,14 @@ export const CardClickable: React.FunctionComponent = () => {
   const [isChecked1, setIsChecked1] = React.useState(false);
   const [isChecked2, setIsChecked2] = React.useState(false);
   const [isChecked3, setIsChecked3] = React.useState(false);
-  const [isSelected1, setIsSelected1] = React.useState(false);
+  const [isClicked, setIsClicked] = React.useState(false);
 
   const id1 = 'clickable-selectable-card-input-1';
   const id2 = 'clickable-selectable-card-input-2';
   const id3 = 'clickable-selectable-card-input-3';
 
   const onClick = () => {
-    setIsSelected1((prevState) => !prevState);
+    setIsClicked((prevState) => !prevState);
   };
 
   const onChange = (event: React.FormEvent<HTMLInputElement>, checked: boolean) => {
@@ -33,13 +33,18 @@ export const CardClickable: React.FunctionComponent = () => {
 
   return (
     <React.Fragment>
-      <Card id="clickable-selectable-card-example-1" isClickable isSelectable isSelected={isSelected1}>
+      <Card
+        id="clickable-selectable-card-example-1"
+        isClickable
+        isClicked={isClicked}
+        isSelectable
+        isSelected={isChecked1}
+      >
         <CardHeader
           selectableActions={{
             selectableActionId: id1,
             selectableActionAriaLabelledby: 'clickable-selectable-card-example-1',
             name: id1,
-            isChecked: isChecked1,
             onChange
           }}
         >
@@ -51,13 +56,12 @@ export const CardClickable: React.FunctionComponent = () => {
         </CardHeader>
         <CardBody>This card performs an action upon clicking the card title and is selectable.</CardBody>
       </Card>
-      <Card id="clickable-selectable-card-example-2" isClickable isSelectable>
+      <Card id="clickable-selectable-card-example-2" isClickable isSelectable isSelected={isChecked2}>
         <CardHeader
           selectableActions={{
             selectableActionId: id2,
             selectableActionAriaLabelledby: 'clickable-selectable-card-example-2',
             name: id2,
-            isChecked: isChecked2,
             onChange
           }}
         >
@@ -71,13 +75,12 @@ export const CardClickable: React.FunctionComponent = () => {
           .
         </CardBody>
       </Card>
-      <Card id="clickable-selectable-card-example-3" isClickable isSelectable isDisabled>
+      <Card id="clickable-selectable-card-example-3" isClickable isSelectable isDisabled isSelected={isChecked3}>
         <CardHeader
           selectableActions={{
             selectableActionId: id3,
             selectableActionAriaLabelledby: 'clickable-selectable-card-example-3',
             name: id3,
-            isChecked: isChecked3,
             onChange
           }}
         >

diff --git a/packages/react-core/src/components/Card/examples/CardSelectable.tsx b/packages/react-core/src/components/Card/examples/CardSelectable.tsx
@@ -28,41 +28,38 @@ export const SelectableCard: React.FunctionComponent = () => {
 
   return (
     <React.Fragment>
-      <Card id="selectable-card-example-1" isSelectable>
+      <Card id="selectable-card-example-1" isSelectable isSelected={isChecked1}>
         <CardHeader
           selectableActions={{
             selectableActionId: id1,
             selectableActionAriaLabelledby: 'selectable-card-example-1',
             name: id1,
-            isChecked: isChecked1,
             onChange
           }}
         >
           <CardTitle>First card</CardTitle>
         </CardHeader>
         <CardBody>This card is selectable.</CardBody>
       </Card>
-      <Card id="selectable-card-example-2" isSelectable>
+      <Card id="selectable-card-example-2" isSelectable isSelected={isChecked2}>
         <CardHeader
           selectableActions={{
             selectableActionId: id2,
             selectableActionAriaLabelledby: 'selectable-card-example-2',
             name: id2,
-            isChecked: isChecked2,
             onChange
           }}
         >
           <CardTitle>Second card</CardTitle>
         </CardHeader>
         <CardBody>This card is selectable.</CardBody>
       </Card>
-      <Card id="selectable-card-example-3" isSelectable isDisabled>
+      <Card id="selectable-card-example-3" isSelectable isDisabled isSelected={isChecked3}>
         <CardHeader
           selectableActions={{
             selectableActionId: id3,
             selectableActionAriaLabelledby: 'selectable-card-example-3',
             name: id3,
-            isChecked: isChecked3,
             onChange
           }}
         >

diff --git a/packages/react-core/src/components/Card/examples/CardSingleSelectable.tsx b/packages/react-core/src/components/Card/examples/CardSingleSelectable.tsx
@@ -2,45 +2,53 @@ import React from 'react';
 import { Card, CardHeader, CardTitle, CardBody } from '@patternfly/react-core';
 
 export const SingleSelectableCard: React.FunctionComponent = () => {
+  const [isChecked, setIsChecked] = React.useState('');
   const id1 = 'single-selectable-card-input-1';
   const id2 = 'single-selectable-card-input-2';
   const id3 = 'single-selectable-card-input-3';
 
+  const onChange = (event: React.FormEvent<HTMLInputElement>) => {
+    setIsChecked(event.currentTarget.id);
+  };
+
   return (
     <React.Fragment>
-      <Card id="single-selectable-card-example-1" isSelectable>
+      <Card id="single-selectable-card-example-1" isSelectable isSelected={isChecked === id1}>
         <CardHeader
           selectableActions={{
             selectableActionId: id1,
             selectableActionAriaLabelledby: 'single-selectable-card-example-1',
             name: 'single-selectable-card-example',
-            variant: 'single'
+            variant: 'single',
+            onChange
           }}
         >
           <CardTitle>First card</CardTitle>
         </CardHeader>
         <CardBody>This card is single selectable.</CardBody>
       </Card>
-      <Card id="single-selectable-card-example-2" isSelectable>
+      <Card id="single-selectable-card-example-2" isSelectable isSelected={isChecked === id2}>
         <CardHeader
           selectableActions={{
             selectableActionId: id2,
             selectableActionAriaLabelledby: 'single-selectable-card-example-2',
             name: 'single-selectable-card-example',
-            variant: 'single'
+            variant: 'single',
+            onChange
           }}
         >
           <CardTitle>Second card</CardTitle>
         </CardHeader>
         <CardBody>This card is single selectable.</CardBody>
       </Card>
-      <Card id="single-selectable-card-example-3" isSelectable isDisabled>
+      <Card id="single-selectable-card-example-3" isSelectable isDisabled isSelected={isChecked === id3}>
         <CardHeader
           selectableActions={{
             selectableActionId: id3,
             selectableActionAriaLabelledby: 'single-selectable-card-example-3',
             name: 'single-selectable-card-example',
-            variant: 'single'
+            variant: 'single',
+            onChange
           }}
         >
           <CardTitle>Third card</CardTitle>

diff --git a/packages/react-integration/cypress/integration/card.spec.ts b/packages/react-integration/cypress/integration/card.spec.ts
@@ -72,10 +72,8 @@ describe('Card Demo Test', () => {
 
   it('Verify clickable only card action is triggered', () => {
     cy.get('#clickable-card-drawer').should('not.have.class', 'pf-m-expanded');
-    cy.get('#clickable-card-example-1 #clickable-card-input-1').should('not.be.checked');
     cy.get('#clickable-card-example-1').click();
     cy.get('#clickable-card-drawer').should('have.class', 'pf-m-expanded');
-    cy.get('#clickable-card-example-1 #clickable-card-input-1').should('be.checked');
   });
 
   it('Verify clickable only card link is navigated to', () => {