code review

sumup-oss · Jan 23, 2025 · 8b37d02 · 8b37d02
1 parent cc0049e
commit 8b37d02
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 42 deletions.
diff --git a/packages/circuit-ui/components/SidePanel/SidePanel.module.css b/packages/circuit-ui/components/SidePanel/SidePanel.module.css
@@ -6,6 +6,19 @@
   box-shadow: none;
 }
 
+.wrapper {
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  width: 100%;
+  height: 100%;
+  padding: 0;
+  padding: env(safe-area-inset-top) env(safe-area-inset-right)
+    env(safe-area-inset-bottom) env(safe-area-inset-left);
+  overflow-y: auto;
+  -webkit-overflow-scrolling: touch;
+}
+
 @media (max-width: 767px) {
   .base {
     top: 0;
@@ -35,6 +48,10 @@
     margin-left: 1px;
   }
 
+  .wrapper {
+    padding-left: 0;
+  }
+
   .content {
     padding: 0 var(--cui-spacings-giga);
   }

diff --git a/packages/circuit-ui/components/SidePanel/SidePanel.spec.tsx b/packages/circuit-ui/components/SidePanel/SidePanel.spec.tsx
@@ -98,6 +98,16 @@ describe('SidePanel', () => {
     await waitFor(() => expect(onClose).toHaveBeenCalled());
   });
 
+  describe('when the panel is not stacked', () => {
+    it('should not show the back button', () => {
+      renderComponent();
+
+      expect(
+        screen.queryByTitle(baseProps.backButtonLabel as string),
+      ).toBeNull();
+    });
+  });
+
   describe('when the panel is stacked', () => {
     it('should show the back button', () => {
       const onBack = vi.fn();
@@ -145,6 +155,16 @@ describe('SidePanel', () => {
     });
   });
 
+  describe('when the panel is on desktop resolution', () => {
+    it('should describe the side panel as modal', () => {
+      const { rerender } = render(<SidePanel {...baseProps} open={false} />);
+      const dialog = screen.getByRole('dialog', { hidden: true });
+      vi.spyOn(dialog, 'show');
+      rerender(<SidePanel {...baseProps} open />);
+      expect(dialog.show).toHaveBeenCalledOnce();
+    });
+  });
+
   describe('when the panel is on mobile resolution', () => {
     it('should describe the side panel as modal', () => {
       const { rerender } = render(

diff --git a/packages/circuit-ui/components/SidePanel/SidePanel.tsx b/packages/circuit-ui/components/SidePanel/SidePanel.tsx
@@ -31,7 +31,6 @@ import { clsx } from '../../styles/clsx.js';
 import { useAnimation } from '../../hooks/useAnimation/index.js';
 import { sharedClasses } from '../../styles/shared.js';
 import { useI18n } from '../../hooks/useI18n/useI18n.js';
-import { useLatest } from '../../hooks/useLatest/index.js';
 
 import { Header } from './components/Header/index.js';
 import type { ChildrenRenderProps, OnBack, OnClose } from './useSidePanel.js';
@@ -94,29 +93,33 @@ export const SidePanel = forwardRef<HTMLDialogElement, SidePanelProps>(
 
       const [, setAnimating] = useAnimation();
 
-      const triggerAnimation = useCallback(
-        (isOpening: boolean, callBack?: OnBack | OnClose) => {
+      const onOpening = useCallback(() => {
+        setAnimating({
+          duration: animationDuration,
+          onStart: () => {
+            setAnimationClass(
+              animationDuration > 0
+                ? sharedClasses.animationSlideRightIn
+                : undefined,
+            );
+          },
+          onEnd: () => {
+            setAnimationClass(undefined);
+          },
+        });
+      }, [animationDuration, setAnimating]);
+
+      const onClosing = useCallback(
+        (callBack?: OnBack | OnClose) => {
           setAnimating({
             duration: animationDuration,
             onStart: () => {
-              if (isOpening) {
-                setAnimationClass(
-                  animationDuration > 0
-                    ? sharedClasses.animationSlideRightIn
-                    : undefined,
-                );
-              } else {
-                void callBack?.();
-                setAnimationClass(sharedClasses.animationSlideRightOut);
-              }
+              void callBack?.();
+              setAnimationClass(sharedClasses.animationSlideRightOut);
             },
             onEnd: () => {
-              if (isOpening) {
-                setAnimationClass(undefined);
-              } else {
-                setAnimationClass(classes.closed);
-                onCloseEnd?.();
-              }
+              setAnimationClass(classes.closed);
+              onCloseEnd?.();
             },
           });
         },
@@ -132,12 +135,10 @@ export const SidePanel = forwardRef<HTMLDialogElement, SidePanelProps>(
         setHeaderSticky(event.currentTarget.scrollTop > 0);
       };
 
-      const triggerAnimationRef = useLatest(triggerAnimation);
-
       useEffect(() => {
         // trigger animation on opening
-        triggerAnimationRef.current(true);
-      }, [triggerAnimationRef]);
+        onOpening();
+      }, [onOpening]);
 
       useEffect(
         () => () => {
@@ -146,23 +147,6 @@ export const SidePanel = forwardRef<HTMLDialogElement, SidePanelProps>(
         [onCloseEnd],
       );
 
-      const content = (
-        <div onScroll={handleScroll}>
-          <Header
-            backButtonLabel={backButtonLabel}
-            closeButtonLabel={closeButtonLabel}
-            headline={headline}
-            id={headerAriaId}
-            onBack={onBack ? () => triggerAnimation(false, onBack) : undefined}
-            onClose={() => triggerAnimation(false, onClose)}
-            isSticky={isHeaderSticky}
-          />
-          <div className={classes.content}>
-            {isFunction(children) ? children({ onBack, onClose }) : children}
-          </div>
-        </div>
-      );
-
       return (
         <Dialog
           {...rest}
@@ -172,12 +156,25 @@ export const SidePanel = forwardRef<HTMLDialogElement, SidePanelProps>(
           aria-labelledby={headerAriaId}
           animationDuration={animationDuration}
           className={clsx(classes.base, animationClass, className)}
-          onCloseStart={() => triggerAnimation(false, onBack || onClose)}
+          onCloseStart={() => onClosing(onBack || onClose)}
           preventOutsideClickClose={true}
           preventEscapeKeyClose={preventEscapeKeyClose}
           hideCloseButton
         >
-          {content}
+          <div className={classes.wrapper} onScroll={handleScroll}>
+            <Header
+              backButtonLabel={backButtonLabel}
+              closeButtonLabel={closeButtonLabel}
+              headline={headline}
+              id={headerAriaId}
+              onBack={onBack ? () => onClosing(onBack) : undefined}
+              onClose={() => onClosing(onClose)}
+              isSticky={isHeaderSticky}
+            />
+            <div className={classes.content}>
+              {isFunction(children) ? children({ onBack, onClose }) : children}
+            </div>
+          </div>
         </Dialog>
       );
     }

diff --git a/packages/circuit-ui/components/SidePanel/SidePanelContext.tsx b/packages/circuit-ui/components/SidePanel/SidePanelContext.tsx
@@ -142,6 +142,10 @@ export function SidePanelProvider({
               resolve();
             }
 
+            dispatch({
+              type: 'update',
+              item: sidePanel,
+            });
             dispatch({
               type: 'remove',
               id: sidePanel.id,