feat(Modal): labelClose prop is now required when onClose is defined

DSil · DSil · commit ef7f01055bb5 · 2025-02-21T11:48:31.000+01:00
BREAKING CHANGE: the labelClose prop no longer has a default value.
It is now required whenever onClose is defined and
hasCloseButton is not explicitly set to false (default value is true).
diff --git a/packages/orbit-components/src/Modal/Modal.ct-story.tsx b/packages/orbit-components/src/Modal/Modal.ct-story.tsx
@@ -13,7 +13,7 @@ const content = "Lorem ispum dolor sit amet.";
 
 export function ModalVisualDefaultStory({ size = SIZES.NORMAL, isMobileFullPage = false }) {
   return (
-    <Modal onClose={() => {}} size={size} isMobileFullPage={isMobileFullPage}>
+    <Modal onClose={() => {}} size={size} isMobileFullPage={isMobileFullPage} labelClose="Close">
       <ModalHeader
         title="Normal header"
         illustration={<Illustration name="AppKiwi" size="small" />}
@@ -36,7 +36,7 @@ export function ModalVisualDefaultStory({ size = SIZES.NORMAL, isMobileFullPage
 
 export function ModalVisualMobileHeader() {
   return (
-    <Modal onClose={() => {}} size={SIZES.NORMAL} mobileHeader>
+    <Modal onClose={() => {}} size={SIZES.NORMAL} mobileHeader labelClose="Close">
       <ModalHeader
         title="Suppressed header"
         illustration={<Illustration name="AppKiwi" size="small" />}
@@ -59,7 +59,12 @@ export function ModalVisualMobileHeader() {
 
 export function ModalVisualNoHeaderNoFooter({ isMobileFullPage = false }) {
   return (
-    <Modal onClose={() => {}} size={SIZES.NORMAL} isMobileFullPage={isMobileFullPage}>
+    <Modal
+      onClose={() => {}}
+      size={SIZES.NORMAL}
+      isMobileFullPage={isMobileFullPage}
+      labelClose="Close"
+    >
       <ModalSection>
         <Text>No Header nor Footer modal</Text>
       </ModalSection>
@@ -69,7 +74,12 @@ export function ModalVisualNoHeaderNoFooter({ isMobileFullPage = false }) {
 
 export function ModalVisualHeaderOnly({ isMobileFullPage = false }) {
   return (
-    <Modal onClose={() => {}} size={SIZES.NORMAL} isMobileFullPage={isMobileFullPage}>
+    <Modal
+      onClose={() => {}}
+      size={SIZES.NORMAL}
+      isMobileFullPage={isMobileFullPage}
+      labelClose="Close"
+    >
       <ModalHeader
         title="Normal header"
         illustration={<Illustration name="AppKiwi" size="small" />}
diff --git a/packages/orbit-components/src/Modal/Modal.stories.tsx b/packages/orbit-components/src/Modal/Modal.stories.tsx
@@ -236,7 +236,7 @@ export const RemovableSections: Story = {
     const { Container, onClose, triggerRef } = useModal();
     return (
       <Container>
-        <Modal triggerRef={triggerRef} onClose={onClose}>
+        <Modal triggerRef={triggerRef} onClose={onClose} labelClose="Close">
           <ModalHeader
             title="Enjoy something to eat while you fly"
             illustration={<Illustration name="Meal" size="small" />}
@@ -340,7 +340,7 @@ export const WithForm: Story = {
 
     return (
       <Container>
-        <Modal triggerRef={triggerRef} onClose={onClose} fixedFooter>
+        <Modal triggerRef={triggerRef} onClose={onClose} labelClose="Close" fixedFooter>
           <ModalHeader title="Refund" description="Reservation number: 123456789" />
           <ModalSection>
             <Stack>
@@ -424,6 +424,7 @@ export const WithItinerary: Story = {
           triggerRef={triggerRef}
           ariaLabel="Itinerary from Prague to Frankfurt"
           onClose={onClose}
+          labelClose="Close"
         >
           <ModalSection>
             <Itinerary>
@@ -607,7 +608,7 @@ export const Rtl: Story = {
     return (
       <Container>
         <RenderInRtl>
-          <Modal triggerRef={triggerRef} onClose={onClose}>
+          <Modal triggerRef={triggerRef} onClose={onClose} labelClose="Close">
             <ModalHeader
               title="The title of the ModalHeader"
               illustration={<Illustration name="Accommodation" size="small" />}
diff --git a/packages/orbit-components/src/Modal/README.md b/packages/orbit-components/src/Modal/README.md
@@ -38,7 +38,7 @@ Table below contains all types of the props available in the Modal component.
 | hasCloseButton      | `boolean`                  | `true`     | Defines whether the Modal displays a close button. If you disable this, we recommend adding some kind of an alternative.                                                                              |
 | disableAnimation    | `boolean`                  | `false`    | Defines whether the Modal performs the slide in animation on mobile. If you want to improve your [CLS](https://web.dev/cls/) score, you might want to set this to `true`.                             |
 | mobileHeader        | `boolean`                  | `true`     | If `false` the ModalHeader will not have MobileHeader and CloseContainer.                                                                                                                             |
-| labelClose          | `string`                   | `Close`    | The label for the close button.                                                                                                                                                                       |
+| labelClose          | `string`                   |            | The label for the close button. It is required all the time, unless `hasCloseButton` is explicitly set to `false`.                                                                                    |
 | onScroll            | `event => void \| Promise` |            | Function for handling `onScroll` event. [See Functional specs](#functional-specs).                                                                                                                    |
 | ariaLabelledby      | `string`                   |            | The `aria-labelledby` attribute of the Modal. It should be used if `title` is not defined on the ModalHeader.                                                                                         |
 | ariaDescribedby     | `string`                   |            | The `aria-describedby` attribute of the Modal. It should be used if `description` is not defined on the ModalHeader.                                                                                  |
diff --git a/packages/orbit-components/src/Modal/__tests__/index.test.tsx b/packages/orbit-components/src/Modal/__tests__/index.test.tsx
@@ -18,7 +18,7 @@ const mockUseMediaQuery = useMediaQuery;
 describe("Modal", () => {
   it("should have expected DOM output", () => {
     render(
-      <Modal dataTest="container">
+      <Modal dataTest="container" labelClose="Close">
         <ModalHeader
           dataTest="header"
           title="modal title"
@@ -52,7 +52,7 @@ describe("Modal", () => {
     jest.useFakeTimers();
 
     render(
-      <Modal>
+      <Modal labelClose="Close">
         <input />
       </Modal>,
     );
@@ -66,7 +66,7 @@ describe("Modal", () => {
   it("should expose ref scrolling API", () => {
     const ref = React.createRef<React.ElementRef<typeof Modal>>();
     render(
-      <Modal ref={ref} dataTest="test">
+      <Modal ref={ref} dataTest="test" labelClose="Close">
         content
       </Modal>,
     );
@@ -80,7 +80,7 @@ describe("Modal", () => {
     const onScroll = jest.fn();
 
     render(
-      <Modal onScroll={onScroll}>
+      <Modal onScroll={onScroll} labelClose="Close">
         <ModalSection>
           <div style={{ height: 500 }}>kek</div>
         </ModalSection>
@@ -101,15 +101,15 @@ describe("Modal", () => {
     // @ts-expect-error jest
     mockUseMediaQuery.mockImplementation(() => ({ isLargeMobile: true }));
     const { rerender } = render(
-      <Modal ref={modalRef} scrollingElementRef={scrollingElementRef}>
+      <Modal ref={modalRef} scrollingElementRef={scrollingElementRef} labelClose="Close">
         content
       </Modal>,
     );
     expect(scrollingElement.current).toBe(modalRef.current?.modalBody.current);
     // @ts-expect-error jest
     mockUseMediaQuery.mockImplementation(() => ({ isLargeMobile: false }));
     rerender(
-      <Modal ref={modalRef} scrollingElementRef={scrollingElementRef}>
+      <Modal ref={modalRef} scrollingElementRef={scrollingElementRef} labelClose="Close">
         content
       </Modal>,
     );
@@ -123,7 +123,7 @@ describe("Modal", () => {
     const onClose = jest.fn();
 
     render(
-      <Modal hasCloseButton onClose={onClose}>
+      <Modal hasCloseButton onClose={onClose} labelClose="Close">
         content
       </Modal>,
     );
@@ -137,13 +137,13 @@ describe("Modal", () => {
 
   it("should have fixed header background overlay when needed", () => {
     const { rerender } = render(
-      <Modal hasCloseButton onClose={() => {}}>
+      <Modal onClose={() => {}} labelClose="Close">
         content
       </Modal>,
     );
     screen.getByTestId("CloseContainer");
     rerender(
-      <Modal>
+      <Modal labelClose="Close">
         <ModalHeader title="title" />
       </Modal>,
     );
diff --git a/packages/orbit-components/src/Modal/__typetests__/index.tsx b/packages/orbit-components/src/Modal/__typetests__/index.tsx
@@ -18,7 +18,7 @@ export default function Test() {
     });
   }, []);
   return (
-    <Modal ref={modalRef} scrollingElementRef={scrollingElementRef}>
+    <Modal ref={modalRef} scrollingElementRef={scrollingElementRef} labelClose="Close">
       content
     </Modal>
   );
diff --git a/packages/orbit-components/src/Modal/types.d.ts b/packages/orbit-components/src/Modal/types.d.ts
@@ -7,28 +7,37 @@ import type * as Common from "../common/types";
 
 type Size = "extraSmall" | "small" | "normal" | "large" | "extraLarge";
 
-export interface Props extends Common.Globals {
-  readonly size?: Size;
-  readonly children: React.ReactNode;
-  readonly triggerRef?: React.RefObject<HTMLElement>;
-  readonly lockScrolling?: boolean;
-  readonly scrollingElementRef?: React.Ref<HTMLElement>;
-  readonly onClose?: Common.Event<
-    | React.KeyboardEvent<HTMLDivElement>
-    | React.SyntheticEvent<HTMLButtonElement | HTMLDivElement | HTMLAnchorElement>
-  >;
-  readonly fixedFooter?: boolean;
-  readonly onScroll?: Common.Event<React.UIEvent<HTMLDivElement>>;
-  readonly mobileHeader?: boolean;
-  readonly isMobileFullPage?: boolean;
-  readonly preventOverlayClose?: boolean;
-  readonly hasCloseButton?: boolean;
-  readonly disableAnimation?: boolean;
-  readonly labelClose?: string;
-  readonly ariaLabel?: string;
-  readonly ariaLabelledby?: string;
-  readonly ariaDescribedby?: string;
-}
+export type closable =
+  | {
+      readonly hasCloseButton?: true;
+      readonly labelClose: string;
+    }
+  | {
+      readonly hasCloseButton: false;
+      readonly labelClose?: string;
+    };
+
+export type Props = Common.Globals &
+  closable & {
+    readonly size?: Size;
+    readonly children: React.ReactNode;
+    readonly triggerRef?: React.RefObject<HTMLElement>;
+    readonly lockScrolling?: boolean;
+    readonly scrollingElementRef?: React.Ref<HTMLElement>;
+    readonly onClose?: Common.Event<
+      | React.KeyboardEvent<HTMLDivElement>
+      | React.SyntheticEvent<HTMLButtonElement | HTMLDivElement | HTMLAnchorElement>
+    >;
+    readonly fixedFooter?: boolean;
+    readonly onScroll?: Common.Event<React.UIEvent<HTMLDivElement>>;
+    readonly mobileHeader?: boolean;
+    readonly isMobileFullPage?: boolean;
+    readonly preventOverlayClose?: boolean;
+    readonly disableAnimation?: boolean;
+    readonly ariaLabel?: string;
+    readonly ariaLabelledby?: string;
+    readonly ariaDescribedby?: string;
+  };
 
 export interface Instance {
   getScrollPosition: () => number | null;
