Merge pull request #6361 from Sage/FE-5369-pod-edit-button

fix(pod): make edit button an HTML button rather than an anchor tag

cypress/components/pod/pod.cy.tsx

@@ -3,6 +3,7 @@ import Pod, { PodProps } from "../../../src/components/pod";
 import {
   PodExample,
   PodDefault,
+  EditExample,
   SoftDeleteExample,
   SoftDeleteExampleWithChildren,
 } from "../../../src/components/pod/pod-test.stories";
@@ -390,7 +391,7 @@ context("Testing Pod component", () => {
     );
 
     it.each(["left", "center", "right"] as PodProps["alignTitle"][])(
-      "should check title alignment accessbility for Pod component when text is aligned to the %s",
+      "should check title alignment accessibility for Pod component when text is aligned to the %s",
       (alignTitle) => {
         CypressMountWithProviders(<PodDefault alignTitle={alignTitle} />);
 
@@ -405,7 +406,7 @@ context("Testing Pod component", () => {
       SIZE.LARGE,
       SIZE.EXTRALARGE,
     ] as PodProps["size"][])(
-      "should check accessbility when size is %s for Pod component",
+      "should check accessibility when size is %s for Pod component",
       (size) => {
         CypressMountWithProviders(<PodDefault size={size} />);
 
@@ -414,7 +415,7 @@ context("Testing Pod component", () => {
     );
 
     it.each([true, false])(
-      "when internalEditButton is %s for Pod component, check accessbility",
+      "when internalEditButton is %s for Pod component, check accessibility",
       (boolVal) => {
         CypressMountWithProviders(<PodDefault internalEditButton={boolVal} />);
 
@@ -433,7 +434,7 @@ context("Testing Pod component", () => {
     );
 
     it.each(specialCharacters)(
-      "should check accessbility when title is %s for Pod component",
+      "should check accessibility when title is %s for Pod component",
       (title) => {
         CypressMountWithProviders(<PodDefault title={title} />);
 
@@ -442,7 +443,7 @@ context("Testing Pod component", () => {
     );
 
     it.each(specialCharacters)(
-      "should check accessbility when subtitle is %s for Pod component",
+      "should check accessibility when subtitle is %s for Pod component",
       (subtitle) => {
         CypressMountWithProviders(<PodDefault subtitle={subtitle} />);
 
@@ -478,16 +479,21 @@ context("Testing Pod component", () => {
       cy.checkAccessibility();
     });
 
+    it("should check accessibility with edit button", () => {
+      CypressMountWithProviders(<EditExample />);
+      cy.checkAccessibility();
+    });
+
     it.each([true, false])(
-      "should check accessbility when softDelete is %s",
+      "should check accessibility when softDelete is %s",
       (boolVal) => {
         CypressMountWithProviders(<SoftDeleteExample softDelete={boolVal} />);
 
         cy.checkAccessibility();
       }
     );
 
-    it("should check accessbility for SoftDelete with chidlren", () => {
+    it("should check accessibility for SoftDelete with children", () => {
       CypressMountWithProviders(<SoftDeleteExampleWithChildren />);
       cy.checkAccessibility();
     });

cypress/locators/pod/locators.js

@@ -5,6 +5,6 @@ export const POD_TITLE = '[data-element="title"]';
 export const POD_SUBTITLE = '[data-element="subtitle"]';
 export const POD_CONTENT = '[data-element="content"]';
 export const POD_FOOTER = '[data-element="footer"]';
-export const POD_EDIT = 'a[data-element="edit"]';
+export const POD_EDIT = 'button[data-element="edit"]';
 export const POD_DELETE = 'button[data-element="delete"]';
 export const POD_UNDO = 'button[data-element="undo"]';

cypress/locators/show-edit-pod/locators.js

@@ -9,7 +9,7 @@ export const SHOW_EDIT_POD_CANCEL_BUTTON =
 export const SHOW_EDIT_POD_SAVE_BUTTON = 'button[data-element="submit-button"]';
 export const SHOW_EDIT_POD_DELETE_BUTTON = "button[data-element=delete-button]";
 export const SHOW_EDIT_POD_TITLE = "h4[data-element=title]";
-export const SHOW_EDIT_POD_EDIT_BUTTON = 'a[data-element="edit"]';
+export const SHOW_EDIT_POD_EDIT_BUTTON = 'button[data-element="edit"]';
 export const SHOW_EDIT_POD_UNDO_BUTTON = "button[data-element=undo]";
 export const SHOW_EDIT_POD_TRANSITION_NAME =
   'div[class="test_cypress-enter-done"]';

src/components/pod/pod-test.stories.tsx

@@ -93,6 +93,18 @@ export const PodDefault = (props: PodProps) => {
   return <Pod title="Title" {...props} />;
 };
 
+export const EditExample = (props: PodProps) => {
+  return (
+    <Pod
+      title="Title"
+      subtitle="Subtitle"
+      footer="Footer"
+      onEdit={() => {}}
+      {...props}
+    />
+  );
+};
+
 export const SoftDeleteExample = (props: PodProps) => {
   return (
     <Pod

src/components/pod/pod.component.tsx

@@ -1,6 +1,5 @@
 import React, { useState, useMemo } from "react";
 import { MarginProps } from "styled-system";
-import Logger from "../../__internal__/utils/logger";
 import useLocale from "../../hooks/__internal__/useLocale";
 
 import {
@@ -45,12 +44,9 @@ export interface PodProps extends MarginProps {
   /** A component to render as a Pod footer */
   footer?: string | React.ReactNode;
   /** Supplies an edit action to the pod */
-  onEdit?:
-    | string
-    | Record<string, unknown>
-    | ((
-        ev: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
-      ) => void);
+  onEdit?: (
+    ev: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
+  ) => void;
   /** Supplies a delete action to the pod */
   onDelete?: (
     ev: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
@@ -73,9 +69,6 @@ export interface PodProps extends MarginProps {
   internalEditButton?: boolean;
 }
 
-let deprecationOnEditStringWarnTriggered = false;
-let deprecationOnEditObjectWarnTriggered = false;
-
 const Pod = React.forwardRef<HTMLDivElement, PodProps>(
   (
     {
@@ -103,20 +96,6 @@ const Pod = React.forwardRef<HTMLDivElement, PodProps>(
     }: PodProps,
     ref
   ) => {
-    if (!deprecationOnEditStringWarnTriggered && typeof onEdit === "string") {
-      deprecationOnEditStringWarnTriggered = true;
-      Logger.deprecate(
-        "Support for passing strings to the `onEdit` prop of the `Pod` component is now deprecated. Please only pass event handlers to `onEdit`."
-      );
-    }
-
-    if (!deprecationOnEditObjectWarnTriggered && typeof onEdit === "object") {
-      deprecationOnEditObjectWarnTriggered = true;
-      Logger.deprecate(
-        "Support for passing objects to the `onEdit` prop of the `Pod` component is now deprecated. Please only pass event handlers to `onEdit`."
-      );
-    }
-
     const [isEditFocused, setEditFocused] = useState(false);
     const [isEditHovered, setEditHovered] = useState(false);
     const [isDeleteFocused, setDeleteFocused] = useState(false);
@@ -158,10 +137,8 @@ const Pod = React.forwardRef<HTMLDivElement, PodProps>(
       onMouseLeave: () => setEditHovered(false),
       onFocus: () => setEditFocused(true),
       onBlur: () => setEditFocused(false),
-      ...(typeof onEdit === "function" && {
-        onClick: processPodAction(onEdit),
-        onKeyDown: processPodAction(onEdit),
-      }),
+      onClick: onEdit && processPodAction(onEdit),
+      onKeyDown: onEdit && processPodAction(onEdit),
     };
 
     return (
@@ -264,7 +241,6 @@ const Pod = React.forwardRef<HTMLDivElement, PodProps>(
                 noBorder={!border}
                 size={size}
                 variant={variant}
-                {...(typeof onEdit === "string" ? { href: onEdit } : onEdit)}
               >
                 <Icon type="edit" />
               </StyledEditAction>

src/components/pod/pod.spec.tsx

@@ -19,7 +19,6 @@ import {
   assertStyleMatch,
   testStyledSystemMargin,
 } from "../../__spec_helper__/test-utils";
-import Logger from "../../__internal__/utils/logger";
 import Typography, { VariantTypes } from "../typography/typography.component";
 
 const variantColors = {
@@ -66,46 +65,6 @@ function assertAbsolutePositioning(
 describe("Pod", () => {
   testStyledSystemMargin((props) => <Pod {...props} />);
 
-  describe("deprecation warnings", () => {
-    let mockConsole: jest.SpyInstance;
-    let loggerSpy: jest.SpyInstance;
-
-    beforeEach(() => {
-      // Mock console.warn to prevent warning from appearing in console while tests are running
-      mockConsole = jest
-        .spyOn(global.console, "warn")
-        .mockImplementation(() => undefined);
-    });
-
-    afterEach(() => {
-      mockConsole.mockRestore();
-    });
-
-    it("when onEdit prop is a string, raise deprecation warning once in the console", () => {
-      loggerSpy = jest.spyOn(Logger, "deprecate");
-      mount(<Pod onEdit="foobar.com">Example Pod</Pod>);
-
-      expect(loggerSpy).toHaveBeenCalledWith(
-        "Support for passing strings to the `onEdit` prop of the `Pod` component is now deprecated. Please only pass event handlers to `onEdit`."
-      );
-      expect(loggerSpy).toHaveBeenCalledTimes(1);
-
-      loggerSpy.mockClear();
-    });
-
-    it("when onEdit prop is an object, raise deprecation warning once in the console", () => {
-      loggerSpy = jest.spyOn(Logger, "deprecate");
-      mount(<Pod onEdit={{ href: "foobar.com" }}>Example Pod</Pod>);
-
-      expect(loggerSpy).toHaveBeenCalledWith(
-        "Support for passing objects to the `onEdit` prop of the `Pod` component is now deprecated. Please only pass event handlers to `onEdit`."
-      );
-      expect(loggerSpy).toHaveBeenCalledTimes(1);
-
-      loggerSpy.mockClear();
-    });
-  });
-
   it("renders children correctly when text %s is passed as a child", () => {
     const text = "Pod content";
     const wrapper = mount(<Pod>{text}</Pod>);
@@ -436,7 +395,7 @@ describe("Pod", () => {
       "when edit button is %s, render edit button and pod with correct colours",
       (_, event) => {
         const wrapper = render({ onEdit: () => {} });
-        wrapper.find("a[data-element='edit']").simulate(event);
+        wrapper.find("button[data-element='edit']").simulate(event);
 
         assertStyleMatch(
           { backgroundColor: "var(--colorsActionMajor600)" },
@@ -512,23 +471,25 @@ describe("Pod", () => {
     it("clicking on the edit button invokes onEdit event handler", () => {
       const onEdit = jest.fn();
       const wrapper = render({ onEdit });
-      wrapper.find('a[data-element="edit"]').simulate("click");
+      wrapper.find('button[data-element="edit"]').simulate("click");
       expect(onEdit).toHaveBeenCalled();
     });
 
     it("pressing enter key on the edit button invokes onEdit", () => {
       const onEdit = jest.fn();
       const wrapper = render({ onEdit });
       wrapper
-        .find('a[data-element="edit"]')
+        .find('button[data-element="edit"]')
         .simulate("keydown", { key: "Enter" });
       expect(onEdit).toHaveBeenCalled();
     });
 
     it("pressing a non-enter key on the edit button does not invoke onEdit", () => {
       const onEdit = jest.fn();
       const wrapper = render({ onEdit });
-      wrapper.find('a[data-element="edit"]').simulate("keydown", { key: "a" });
+      wrapper
+        .find('button[data-element="edit"]')
+        .simulate("keydown", { key: "a" });
       expect(onEdit).not.toHaveBeenCalled();
     });
 
@@ -550,7 +511,7 @@ describe("Pod", () => {
               displayEditButtonOnHover,
               triggerEditOnContent,
             });
-            wrapper.find('a[data-element="edit"]').simulate(event);
+            wrapper.find('button[data-element="edit"]').simulate(event);
             assertStyleMatch(
               { backgroundColor: "var(--colorsActionMajor600)" },
               wrapper.find(StyledBlock)
@@ -571,7 +532,7 @@ describe("Pod", () => {
               displayEditButtonOnHover,
               triggerEditOnContent,
             });
-            wrapper.find('a[data-element="edit"]').simulate(event);
+            wrapper.find('button[data-element="edit"]').simulate(event);
             assertStyleMatch(
               { backgroundColor: variantColors[variant].pod },
               wrapper.find(StyledBlock)

src/components/pod/pod.style.ts

@@ -198,7 +198,7 @@ interface StyledEditActionProps extends CommonPodButtonProps {
   displayOnlyOnHover?: boolean;
 }
 
-const StyledEditAction = styled.a<StyledEditActionProps>`
+const StyledEditAction = styled(IconButton)<StyledEditActionProps>`
   && {
     ${({
       displayOnlyOnHover,