Merge branch 'main' into fix/dropdown-catch-null-value

Frontify · Jan 21, 2025 · cb95d7e · cb95d7e
2 parents 2dec00a + 57d7a63
commit cb95d7e
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 69 deletions.
diff --git a/.changeset/fast-eyes-lay.md b/.changeset/fast-eyes-lay.md
@@ -0,0 +1,5 @@
+---
+"@frontify/fondue-components": patch
+---
+
+feat(`Tooltip`): add button type property to prevent form submission
diff --git a/.changeset/wild-carrots-fix.md b/.changeset/wild-carrots-fix.md
@@ -0,0 +1,5 @@
+---
+"@frontify/fondue-components": patch
+---
+
+feat(Dropdown): enforce compact paddings
diff --git a/packages/components/src/components/Box/Box.stories.tsx b/packages/components/src/components/Box/Box.stories.tsx
@@ -17,8 +17,8 @@ const meta: Meta<typeof Box> = {
         },
     },
     args: {
-        width: '100px',
-        height: '100px',
+        width: 25,
+        height: 25,
     },
 };
 export default meta;
@@ -33,10 +33,10 @@ export const Default: Story = {
     },
 };
 
-export const WithSizeToken: Story = {
+export const PixelValues: Story = {
     args: {
-        width: 25,
-        height: 25,
+        width: '100px',
+        height: '100px',
     },
     render: (args) => {
         return (

diff --git a/packages/components/src/components/Dropdown/Dropdown.stories.tsx b/packages/components/src/components/Dropdown/Dropdown.stories.tsx
@@ -369,27 +369,6 @@ export const RightSide: Story = {
     ),
 };
 
-export const CompactPadding: Story = {
-    render: ({ ...args }) => (
-        <Dropdown.Root {...args}>
-            <Dropdown.Trigger>
-                <Button>Trigger</Button>
-            </Dropdown.Trigger>
-            <Dropdown.Content padding="compact" side="right">
-                <Dropdown.Item onSelect={() => {}}>
-                    <p>Item 1</p>
-                    <p>Item 1 description</p>
-                </Dropdown.Item>
-                <Dropdown.Item onSelect={() => {}}>
-                    <p>Item 2</p>
-                    <p>Item 2 description</p>
-                </Dropdown.Item>
-                <Dropdown.Item onSelect={() => {}}>Item 3</Dropdown.Item>
-            </Dropdown.Content>
-        </Dropdown.Root>
-    ),
-};
-
 export const WithTooltip: Story = {
     render: ({ ...args }) => (
         <Dropdown.Root {...args}>

diff --git a/packages/components/src/components/Dropdown/Dropdown.tsx b/packages/components/src/components/Dropdown/Dropdown.tsx
@@ -67,11 +67,6 @@ DropdownTrigger.displayName = 'Dropdown.Trigger';
 export type DropdownContentProps = {
     children?: ReactNode;
     'data-test-id'?: string;
-    /**
-     * The vertical padding around each dropdown item.
-     * @default "comfortable"
-     */
-    padding?: 'comfortable' | 'compact';
     /**
      * Defines the alignment of the dropdown.
      * @default "start"
@@ -91,7 +86,6 @@ export type DropdownContentProps = {
 export const DropdownContent = (
     {
         side = 'bottom',
-        padding = 'comfortable',
         align = 'start',
         children,
         preventTriggerFocusOnClose,
@@ -107,7 +101,6 @@ export const DropdownContent = (
                 sideOffset={8}
                 side={side}
                 className={styles.content}
-                data-padding={padding}
                 data-test-id={dataTestId}
                 ref={ref}
                 onCloseAutoFocus={(event) => {
@@ -164,31 +157,17 @@ export const DropdownSubTrigger = (
 DropdownSubTrigger.displayName = 'Dropdown.SubTrigger';
 
 export type DropdownSubContentProps = {
-    /**
-     * The vertical padding around each dropdown item.
-     * @default "comfortable"
-     */
-    padding?: 'comfortable' | 'compact';
     children: ReactNode;
     'data-test-id'?: string;
 };
 
 export const DropdownSubContent = (
-    {
-        padding = 'comfortable',
-        children,
-        'data-test-id': dataTestId = 'fondue-dropdown-subcontent',
-    }: DropdownSubContentProps,
+    { children, 'data-test-id': dataTestId = 'fondue-dropdown-subcontent' }: DropdownSubContentProps,
     ref: ForwardedRef<HTMLDivElement>,
 ) => {
     return (
         <RadixDropdown.Portal>
-            <RadixDropdown.SubContent
-                className={styles.subContent}
-                data-padding={padding}
-                data-test-id={dataTestId}
-                ref={ref}
-            >
+            <RadixDropdown.SubContent className={styles.subContent} data-test-id={dataTestId} ref={ref}>
                 {children}
             </RadixDropdown.SubContent>
         </RadixDropdown.Portal>

diff --git a/packages/components/src/components/Dropdown/styles/dropdown.module.scss b/packages/components/src/components/Dropdown/styles/dropdown.module.scss
@@ -21,14 +21,9 @@
         border-color: var(--line-color-x-strong);
     }
 
-    &[data-padding='compact'] .item,
-    &[data-padding='compact'] .subTrigger {
-        padding: sizeToken(2) sizeToken(5);
-    }
-
-    &[data-padding='comfortable'] .item,
-    &[data-padding='comfortable'] .subTrigger {
-        padding: sizeToken(3) sizeToken(5);
+    .item,
+    .subTrigger {
+        padding: sizeToken(2) sizeToken(3);
     }
 
     @include sm {
@@ -47,8 +42,8 @@
     @include transition-colors;
     display: flex;
     align-items: center;
-    gap: sizeToken(1.5);
-    margin: sizeToken(1);
+    gap: sizeToken(1);
+    margin: sizeToken(0.5) sizeToken(2);
     border-radius: sizeToken(1);
     cursor: pointer;
     text-align: start;
@@ -57,6 +52,14 @@
     color: var(--text-color-weak);
     outline-style: none;
 
+    &:first-child {
+        margin-top: sizeToken(2);
+    }
+
+    &:last-child {
+        margin-bottom: sizeToken(2);
+    }
+
     &[data-state='open'] {
         background-color: var(--box-neutral-color);
         color: var(--text-color);

diff --git a/packages/components/src/components/Flex/Flex.stories.tsx b/packages/components/src/components/Flex/Flex.stories.tsx
@@ -14,6 +14,7 @@ const DecorativeBox = (props: ComponentProps<typeof Box>) => (
         <DecorativeContent />
     </Box>
 );
+DecorativeBox.displayName = 'DecorativeBox';
 
 type Story = StoryObj<typeof Flex>;
 const meta: Meta<typeof Flex> = {
@@ -26,7 +27,7 @@ const meta: Meta<typeof Flex> = {
         },
     },
     args: {
-        gap: '20px',
+        gap: 4,
         direction: 'column',
     },
 };
@@ -36,14 +37,14 @@ export const Default: Story = {
     render: (args) => {
         return (
             <Flex {...args}>
-                <Flex gapX="10px">
+                <Flex gapX={3}>
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
                 </Flex>
-                <Flex gapX="10px">
+                <Flex gapX={3}>
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
@@ -55,11 +56,37 @@ export const Default: Story = {
     },
 };
 
+export const PixelValues: Story = {
+    render: (args) => {
+        return (
+            <Flex {...args}>
+                <Flex gapX="12px">
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                </Flex>
+                <Flex gapX="12px">
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                    <DecorativeBox />
+                </Flex>
+            </Flex>
+        );
+    },
+    args: {
+        gap: '16px',
+    },
+};
+
 export const Nested: Story = {
     render: (args) => {
         return (
             <Flex {...args}>
-                <Flex gapX="10px">
+                <Flex gapX={3}>
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
@@ -73,13 +100,13 @@ export const Nested: Story = {
                         <DecorativeBox />
                     </Flex>
                 </Flex>
-                <Flex p="20px">
+                <Flex p={5}>
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
                     <DecorativeBox />
-                    <Flex gapX="10px">
+                    <Flex gapX={3}>
                         <DecorativeBox />
                         <DecorativeBox />
                         <DecorativeBox />
@@ -104,7 +131,7 @@ export const Responsive: Story = {
             >
                 <Flex
                     gapX={{ sm: '10px' }}
-                    gapY={{ base: '10px', sm: '0px' }}
+                    gapY={{ base: '10px', sm: 0 }}
                     direction={{
                         base: 'column',
                         sm: 'row',
@@ -119,7 +146,7 @@ export const Responsive: Story = {
 
                 <Flex
                     gapX={{ sm: '10px' }}
-                    gapY={{ base: '10px', sm: '0px' }}
+                    gapY={{ base: '10px', sm: 0 }}
                     direction={{
                         base: 'column',
                         sm: 'row',

diff --git a/packages/components/src/components/Grid/Grid.stories.tsx b/packages/components/src/components/Grid/Grid.stories.tsx
@@ -14,6 +14,7 @@ const DecorativeBox = (props: ComponentProps<typeof Box>) => (
         <DecorativeContent />
     </Box>
 );
+DecorativeBox.displayName = 'DecorativeBox';
 
 type Story = StoryObj<typeof Grid>;
 const meta: Meta<typeof Grid> = {
@@ -27,7 +28,7 @@ const meta: Meta<typeof Grid> = {
     },
     args: {
         columns: 'repeat(3, 1fr)',
-        gap: '16px',
+        gap: 4,
     },
 };
 export default meta;

diff --git a/packages/components/src/components/Tooltip/Tooltip.tsx b/packages/components/src/components/Tooltip/Tooltip.tsx
@@ -50,7 +50,12 @@ export const TooltipTrigger = (
     ref: ForwardedRef<HTMLButtonElement>,
 ) => {
     return (
-        <RadixTooltip.Trigger data-test-id={dataTestId} asChild={asChild} ref={ref}>
+        <RadixTooltip.Trigger
+            data-test-id={dataTestId}
+            type={!asChild ? 'button' : undefined}
+            asChild={asChild}
+            ref={ref}
+        >
             {children}
         </RadixTooltip.Trigger>
     );

diff --git a/packages/components/src/components/Tooltip/__tests__/Tooltip.ct.tsx b/packages/components/src/components/Tooltip/__tests__/Tooltip.ct.tsx
@@ -3,13 +3,17 @@
 import { expect, test } from '@playwright/experimental-ct-react';
 import * as sinon from 'sinon';
 
+import { Button } from '#/components/Button/Button';
+
 import { Tooltip } from '../Tooltip';
 
 const TOOLTIP_TRIGGER_TEST_ID = 'fondue-tooltip-trigger';
 const TOOLTIP_CONTENT_TEST_ID = 'fondue-tooltip-content';
 const TOOLTIP_TEXT =
     'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.';
 
+const BUTTON_TEST_ID = 'fondue-button';
+
 test('should render without error', async ({ mount }) => {
     const component = await mount(
         <Tooltip.Root>
@@ -326,3 +330,44 @@ test('should trigger callback on open state change', async ({ mount }) => {
     await component.hover();
     expect(onOpenChange.callCount).toBe(1);
 });
+
+test('should not submit form when clicking TooltipTrigger', async ({ mount, page }) => {
+    const onSubmit = sinon.spy();
+    const component = await mount(
+        <form onSubmit={onSubmit}>
+            <Tooltip.Root>
+                <Tooltip.Trigger data-test-id={TOOLTIP_TRIGGER_TEST_ID}>Click me in form</Tooltip.Trigger>
+                <Tooltip.Content data-test-id={TOOLTIP_CONTENT_TEST_ID}>{TOOLTIP_TEXT}</Tooltip.Content>
+            </Tooltip.Root>
+        </form>,
+    );
+
+    await expect(component).toBeVisible();
+    const trigger = page.getByTestId(TOOLTIP_TRIGGER_TEST_ID);
+    await trigger.click();
+    expect(onSubmit.callCount).toBe(0);
+});
+
+test('should submit form when clicking TooltipTrigger with a Button type submit as trigger', async ({
+    mount,
+    page,
+}) => {
+    const onSubmit = sinon.spy();
+    const component = await mount(
+        <form onSubmit={onSubmit}>
+            <Tooltip.Root>
+                <Tooltip.Trigger asChild>
+                    <Button data-test-id={BUTTON_TEST_ID} type="submit">
+                        Click me in form
+                    </Button>
+                </Tooltip.Trigger>
+                <Tooltip.Content data-test-id={TOOLTIP_CONTENT_TEST_ID}>{TOOLTIP_TEXT}</Tooltip.Content>
+            </Tooltip.Root>
+        </form>,
+    );
+
+    await expect(component).toBeVisible();
+    const trigger = page.getByTestId(BUTTON_TEST_ID);
+    await trigger.click();
+    expect(onSubmit.callCount).toBe(1);
+});