fix(option): makes value and text props optional

Updates Option interface to allow `value` and `text` to be optional to allow users
to compose `children` for complex layouts. This ensures that `SelectList` height
is set relatively when a custom option is passed as child so that there is no
overflow

fix #6939

src/components/select/__internal__/select-list/select-list.component.tsx

@@ -280,11 +280,12 @@ const SelectList = React.forwardRef(
 
     const optionChildrenList = useMemo(
       () =>
-        childrenList.filter(
-          (child) =>
+        childrenList.filter((child) => {
+          return (
             React.isValidElement(child) &&
             (child.type === Option || child.type === OptionRow)
-        ),
+          );
+        }),
       [childrenList]
     );
 

src/components/select/__internal__/utils/with-filter.hoc.tsx

@@ -150,7 +150,9 @@ const withFilter = <T extends WrappedComponentProps>(
             }
 
             return (
-              <StyledOption>{noResultsMessage || noResultsText}</StyledOption>
+              <StyledOption isInteractive>
+                {noResultsMessage || noResultsText}
+              </StyledOption>
             );
           }
 

src/components/select/option/__snapshots__/option.spec.tsx.snap

@@ -2,7 +2,6 @@
 
 exports[`Option renders properly 1`] = `
 .c0 {
-  cursor: pointer;
   box-sizing: border-box;
   line-height: 16px;
   padding: 12px 16px;
@@ -15,6 +14,7 @@ exports[`Option renders properly 1`] = `
   top: 0;
   left: 0;
   width: 100%;
+  cursor: pointer;
   background-color: var(--colorsUtilityMajor200);
 }
 
@@ -36,7 +36,6 @@ exports[`Option renders properly 1`] = `
 
 exports[`Option when no children are provided renders with the text prop as children 1`] = `
 .c0 {
-  cursor: pointer;
   box-sizing: border-box;
   line-height: 16px;
   padding: 12px 16px;
@@ -49,6 +48,7 @@ exports[`Option when no children are provided renders with the text prop as chil
   top: 0;
   left: 0;
   width: 100%;
+  cursor: pointer;
   background-color: var(--colorsUtilityMajor200);
 }
 

src/components/select/option/option.component.tsx

@@ -16,11 +16,11 @@ export interface OptionProps
    */
   id?: string;
   /** The option's visible text, displayed within `<Textbox>` of `<Select>`, and used for filtering */
-  text: string;
-  /** Optional: alternative rendered content, displayed within `<SelectList>` of `<Select>` (eg: an icon, an image, etc) */
+  text?: string;
+  /** Alternative rendered content, displayed within `<SelectList>` of `<Select>` (eg: an icon, an image, etc) */
   children?: React.ReactNode;
-  /** The option's invisible internal value */
-  value: string | Record<string, unknown>;
+  /** The option's invisible internal value, if this is not passed the option will not be treated as interactive or selectable */
+  value?: string | Record<string, unknown>;
   /** MultiSelect only - custom Pill border color - provide any color from palette or any valid css color value. */
   borderColor?: string;
   /** MultiSelect only - fill Pill background with color */
@@ -69,12 +69,12 @@ const Option = React.forwardRef(
     let isSelected = selectListContext.currentOptionsListIndex === index;
     const internalIdRef = useRef(id || guid());
 
-    if (selectListContext.multiselectValues) {
+    if (selectListContext.multiselectValues && value) {
       isSelected = selectListContext.multiselectValues.includes(value);
     }
 
     function handleClick() {
-      if (disabled) {
+      if (disabled || !value) {
         return;
       }
       if (!onClick) {
@@ -98,6 +98,7 @@ const Option = React.forwardRef(
         role="option"
         hidden={hidden}
         style={style}
+        isInteractive={!!value}
         {...{ ...rest, fill: undefined }}
       >
         {children || text}

src/components/select/option/option.spec.tsx

@@ -1,6 +1,8 @@
 import React from "react";
 import TestRenderer from "react-test-renderer";
 import { shallow, mount } from "enzyme";
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
 import Option, { OptionProps } from ".";
 import SelectListContext from "../__internal__/select-list/select-list.context";
 import guid from "../../../__internal__/utils/helpers/guid";
@@ -70,7 +72,7 @@ describe("Option", () => {
       const wrapper = renderOption(props, mount);
       wrapper.simulate("click");
 
-      expect(onSelect).not.toBeCalled();
+      expect(onSelect).not.toHaveBeenCalled();
     });
   });
 
@@ -108,7 +110,7 @@ describe("Option", () => {
   });
 
   describe("when the element is clicked", () => {
-    it("then onSelect should be called with text and value", () => {
+    it("calls onSelect with text and value", () => {
       const onSelect = jest.fn();
       const props = { value: "1", text: "foo", onSelect };
       const wrapper = renderOption(props, mount);
@@ -120,6 +122,38 @@ describe("Option", () => {
         id: mockedGuid,
       });
     });
+
+    it("both onClick and onSelect should be called if both are passed", async () => {
+      const user = userEvent.setup();
+      const onSelect = jest.fn();
+      const onClick = jest.fn();
+      const props = { value: "1", text: "foo", onSelect, onClick };
+      render(
+        <ul>
+          <Option {...props}>Foo</Option>
+        </ul>
+      );
+      await user.click(screen.getByRole("option"));
+
+      expect(onSelect).toHaveBeenCalled();
+      expect(onClick).toHaveBeenCalledWith(props.value);
+    });
+
+    it("neither onClick and onSelect should be called if both are passed but no value is set", async () => {
+      const user = userEvent.setup();
+      const onSelect = jest.fn();
+      const onClick = jest.fn();
+      const props = { text: "foo", onSelect, onClick };
+      render(
+        <ul>
+          <Option {...props}>Foo</Option>
+        </ul>
+      );
+      await user.click(screen.getByRole("option"));
+
+      expect(onSelect).not.toHaveBeenCalled();
+      expect(onClick).not.toHaveBeenCalled();
+    });
   });
 
   describe("tags on component", () => {
@@ -142,3 +176,13 @@ describe("Option", () => {
     });
   });
 });
+
+test("should not render with cursor style pointer when no value or text is passed", () => {
+  render(
+    <ul>
+      <Option>Foo</Option>
+    </ul>
+  );
+
+  expect(screen.getByRole("option")).not.toHaveStyle("cursor: pointer");
+});

src/components/select/option/option.style.ts

@@ -4,10 +4,10 @@ import { OptionProps } from ".";
 interface StyledOptionProps extends Pick<OptionProps, "id"> {
   isHighlighted?: boolean;
   isDisabled?: boolean;
+  isInteractive: boolean;
 }
 
 const StyledOption = styled.li<StyledOptionProps>`
-  cursor: pointer;
   box-sizing: border-box;
   line-height: 16px;
   padding: 12px 16px;
@@ -18,18 +18,21 @@ const StyledOption = styled.li<StyledOptionProps>`
   left: 0;
   width: 100%;
 
-  ${({ isHighlighted }) =>
-    isHighlighted &&
+  ${({ isInteractive, isHighlighted }) =>
+    isInteractive &&
     css`
-      background-color: var(--colorsUtilityMajor200);
+      cursor: pointer;
+      :hover {
+        background-color: var(--colorsUtilityMajor100);
+      }
+      ${isHighlighted &&
+      css`
+        background-color: var(--colorsUtilityMajor200);
+      `}
     `}
 
   ${({ hidden }) => hidden && "display: none;"}
 
-  :hover {
-    background-color: var(--colorsUtilityMajor100);
-  }
-
   ${({ isDisabled }) =>
     isDisabled &&
     css`

src/components/select/simple-select/components.test-pw.tsx

@@ -603,3 +603,52 @@ export const WithObjectAsValue = () => {
     </>
   );
 };
+
+export const ComplexCustomChildren = () => {
+  return (
+    <Box height={220}>
+      <Select
+        mb={0}
+        key="key"
+        id="id"
+        label="Select"
+        aria-label="aria label"
+        name="name"
+        value="value"
+        isLoading={false}
+        readOnly={false}
+        placeholder="placeholder"
+        onChange={() => {}}
+        onOpen={() => {}}
+        onListScrollBottom={() => {}}
+      >
+        <Option>
+          <Box
+            width="100%"
+            display="flex"
+            alignItems="center"
+            justifyContent="center"
+            mt={2}
+            mb={3}
+            flexDirection="column"
+            as="span"
+          >
+            <Box display="flex" mx={2}>
+              <Icon type="error" color="errorRed" />
+              <Box ml={1} width="100%">
+                <Box mb={1}>
+                  <Typography variant="b" color="errorRed">
+                    Something went wrong
+                  </Typography>
+                </Box>
+                <Typography variant="p" color="errorRed" mb={0}>
+                  We couldn't load the data. Please try again later.
+                </Typography>
+              </Box>
+            </Box>
+          </Box>
+        </Option>
+      </Select>
+    </Box>
+  );
+};

src/components/select/simple-select/simple-select-test.stories.tsx

@@ -671,3 +671,51 @@ export const SimpleSelectWithTruncatedText = () => {
     </Select>
   );
 };
+
+export const ComplexCustomChildren = () => {
+  return (
+    <Box height={220}>
+      <Select
+        mb={0}
+        key="key"
+        id="id"
+        label="Select"
+        aria-label="aria label"
+        name="name"
+        value="value"
+        isLoading={false}
+        readOnly={false}
+        placeholder="placeholder"
+        onChange={() => {}}
+        onOpen={() => {}}
+        onListScrollBottom={() => {}}
+      >
+        <Option>
+          <Box
+            width="100%"
+            display="flex"
+            alignItems="center"
+            justifyContent="center"
+            mt={2}
+            mb={3}
+            flexDirection="column"
+          >
+            <Box display="flex" mx={2}>
+              <Icon type="error" color="errorRed" />
+              <Box ml={1} width="100%">
+                <Box mb={1}>
+                  <Typography variant="b" color="errorRed">
+                    Something went wrong
+                  </Typography>
+                </Box>
+                <Typography variant="p" color="errorRed" mb={0}>
+                  We couldn't load the data. Please try again later.
+                </Typography>
+              </Box>
+            </Box>
+          </Box>
+        </Option>
+      </Select>
+    </Box>
+  );
+};

src/components/select/simple-select/simple-select.mdx

@@ -117,6 +117,10 @@ This prop will be called every time a user scrolls to the bottom of the list.
 
 ### Custom Option content
 
+It is also possible to render non-interactive content within the `Option` component.
+This can be achieved by passing `children` with no `value` or `text` props set. However, 
+we recommend checking that there are no accessibility issues with this approach before using it.
+
 <Canvas of={SimpleSelectStories.CustomOptionChildren} />
 
 ### With multiple columns

src/components/select/simple-select/simple-select.pw.tsx

@@ -18,6 +18,7 @@ import {
   SelectWithDynamicallyAddedOption,
   SimpleSelectControlled,
   WithObjectAsValue,
+  ComplexCustomChildren,
 } from "./components.test-pw";
 import {
   commonDataElementInputPreview,
@@ -1055,6 +1056,26 @@ test.describe("SimpleSelect component", () => {
     await expect(selectInput(page)).toHaveCSS("border-radius", "4px");
     await expect(selectListWrapper(page)).toHaveCSS("border-radius", "4px");
   });
+
+  test("should display the custom option in full when list is opened", async ({
+    mount,
+    page,
+  }) => {
+    await mount(<ComplexCustomChildren />);
+
+    await selectText(page).click();
+    const customOptionContent = selectListCustomChild(page, 1).nth(0);
+    const wrapper = selectListWrapper(page);
+    const customOptionHeight = await selectList(page)
+      .locator("li")
+      .evaluate((element) => element.getBoundingClientRect().height);
+    const wrapperHeight = await wrapper.evaluate(
+      (element) => element.getBoundingClientRect().height
+    );
+
+    await expect(customOptionContent).toBeVisible();
+    expect(wrapperHeight).toBe(customOptionHeight);
+  });
 });
 
 test.describe("Check height of Select list when opened", () => {