From f8fa2c6a8dae998b5dbe4acc0465541f2c50f6e2 Mon Sep 17 00:00:00 2001
From: Max Korsunov <maxkors2014@gmail.com>
Date: Thu, 15 Aug 2024 10:41:24 +0200
Subject: [PATCH] fix(ui): #prax-156: update popover based on the comments

---
 packages/ui/src/PenumbraUIProvider/theme.ts | 19 +++++++++++++-
 packages/ui/src/Popover/index.stories.tsx   |  6 ++---
 packages/ui/src/Popover/index.tsx           | 29 ++++++++++++++-------
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/packages/ui/src/PenumbraUIProvider/theme.ts b/packages/ui/src/PenumbraUIProvider/theme.ts
index 465765861f..cb50c923ca 100644
--- a/packages/ui/src/PenumbraUIProvider/theme.ts
+++ b/packages/ui/src/PenumbraUIProvider/theme.ts
@@ -104,6 +104,23 @@ const PALETTE = {
   },
 };
 
+/**
+ * Call `theme.spacing(x)`, where `x` is the number of spacing units (in the
+ * Penumbra theme, 1 spacing unit = 4px) that you want to interpolate into your
+ * CSS or JavaScript. By default, returns a string with the number of pixels
+ * suffixed with `px` -- e.g., `theme.spacing(4)` returns `'16px'`. Pass
+ * `number` as the second argument to get back a number of pixels -- e.g.,
+ * `theme.spacing(4, 'number')` returns `16`.
+ */
+function spacing(spacingUnits: number, returnType?: 'string'): string;
+function spacing(spacingUnits: number, returnType: 'number'): number;
+function spacing(spacingUnits: number, returnType?: 'string' | 'number'): string | number {
+  if (returnType === 'number') {
+    return spacingUnits * 4;
+  }
+  return `${spacingUnits * 4}px`;
+}
+
 export const theme = {
   blur: {
     none: '0px',
@@ -238,7 +255,7 @@ export const theme = {
     textSm: '1.25rem',
     textXs: '1rem',
   },
-  spacing: (spacingUnits: number) => `${spacingUnits * 4}px`,
+  spacing,
   zIndex: {
     disabledOverlay: 10,
     dialogOverlay: 1000,
diff --git a/packages/ui/src/Popover/index.stories.tsx b/packages/ui/src/Popover/index.stories.tsx
index 3ebb171af9..59b09bb2a6 100644
--- a/packages/ui/src/Popover/index.stories.tsx
+++ b/packages/ui/src/Popover/index.stories.tsx
@@ -8,7 +8,7 @@ import styled from 'styled-components';
 import { Shield } from 'lucide-react';
 import { Density } from '../Density';
 
-const WhiteTextWrapper = styled.div`
+const Wrapper = styled.div`
   display: flex;
   flex-direction: column;
   gap: ${props => props.theme.spacing(4)};
@@ -44,7 +44,7 @@ export const Basic: Story = {
         </Popover.Trigger>
 
         <Popover.Content>
-          <WhiteTextWrapper>
+          <Wrapper>
             <Text body as='h3'>
               This is a heading
             </Text>
@@ -59,7 +59,7 @@ export const Basic: Story = {
                 </Button>
               </Density>
             </div>
-          </WhiteTextWrapper>
+          </Wrapper>
         </Popover.Content>
       </Popover>
     );
diff --git a/packages/ui/src/Popover/index.tsx b/packages/ui/src/Popover/index.tsx
index 89a148dd55..ebe0c37f82 100644
--- a/packages/ui/src/Popover/index.tsx
+++ b/packages/ui/src/Popover/index.tsx
@@ -1,16 +1,16 @@
 import { ReactNode } from 'react';
 import * as RadixPopover from '@radix-ui/react-popover';
 import type { PopoverContentProps as RadixPopoverContentProps } from '@radix-ui/react-popover';
-import styled, { keyframes } from 'styled-components';
+import styled, { keyframes, useTheme } from 'styled-components';
 
-const appearAnimation = (spacing: string) => keyframes`
+const scaleIn = keyframes`
   from {
     opacity: 0;
-    transform: translate(${spacing}, ${spacing});
+    transform: scale(0);
   }
   to {
     opacity: 1;
-    transform: translate(0, 0);
+    transform: scale(1);
   }
 `;
 
@@ -18,14 +18,18 @@ const RadixContent = styled.div`
   display: flex;
   flex-direction: column;
   gap: ${props => props.theme.spacing(4)};
+
   width: 240px;
   max-width: 320px;
   padding: ${props => props.theme.spacing(3)} ${props => props.theme.spacing(2)};
-  border-radius: ${props => props.theme.borderRadius.sm};
-  border: 1px solid ${props => props.theme.color.other.tonalStroke};
+
   background: ${props => props.theme.color.other.dialogBackground};
+  border: 1px solid ${props => props.theme.color.other.tonalStroke};
+  border-radius: ${props => props.theme.borderRadius.sm};
   backdrop-filter: blur(${props => props.theme.blur.lg});
-  animation: ${props => appearAnimation(props.theme.spacing(1))} 0.15s ease-out;
+
+  transform-origin: var(--radix-tooltip-content-transform-origin);
+  animation: ${scaleIn} 0.15s ease-out;
 `;
 
 interface ControlledPopoverProps {
@@ -44,7 +48,7 @@ interface ControlledPopoverProps {
 }
 
 interface UncontrolledPopoverProps {
-  isOpen?: false | undefined;
+  isOpen?: undefined;
   onClose?: undefined;
 }
 
@@ -147,9 +151,16 @@ export interface PopoverContentProps {
  * `side` and `align` props.
  */
 const Content = ({ children, side, align }: PopoverContentProps) => {
+  const theme = useTheme();
+
   return (
     <RadixPopover.Portal>
-      <RadixPopover.Content sideOffset={4} side={side} align={align} asChild>
+      <RadixPopover.Content
+        sideOffset={theme.spacing(1, 'number')}
+        side={side}
+        align={align}
+        asChild
+      >
         <RadixContent>{children}</RadixContent>
       </RadixPopover.Content>
     </RadixPopover.Portal>