From 2d9917f0a54c527a4873f3d16901b78c699e4816 Mon Sep 17 00:00:00 2001 From: Osama Abdul Latif <62595605+OsamaAbdellateef@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:45:15 +0300 Subject: [PATCH] [DST-462]Refactor component use Grid Areas (#4164) * Refactor dialog component * Fixing tests * Adding changeset * Adding more tests for content footer and actions sub-components * Deleting stack wrapper from withdialogcontroller story * Update changeset * Update docs examples * Remove the gap * Adding badge * Editing changeset * Adding more info in the changeset about changing dialog.headline to dialog.title --- .changeset/early-sloths-work.md | 16 +++ .../overlay/dialog/dialog-dismiss.demo.tsx | 8 +- .../overlay/dialog/dialog-form.demo.tsx | 33 ++--- .../components/overlay/dialog/dialog.mdx | 11 +- .../components/src/Dialog/Dialog.stories.tsx | 120 ++++++++---------- .../components/src/Dialog/Dialog.test.tsx | 91 ++++++++++--- packages/components/src/Dialog/Dialog.tsx | 66 ++++++++-- .../theme-b2b/src/components/Dialog.styles.ts | 2 +- .../src/components/Dialog.styles.ts | 2 +- 9 files changed, 226 insertions(+), 123 deletions(-) create mode 100644 .changeset/early-sloths-work.md diff --git a/.changeset/early-sloths-work.md b/.changeset/early-sloths-work.md new file mode 100644 index 0000000000..004b80213f --- /dev/null +++ b/.changeset/early-sloths-work.md @@ -0,0 +1,16 @@ +--- +'@marigold/components': major +'@marigold/theme-core': major +'@marigold/theme-b2b': major +--- + +**Breaking changes** + +- `Dialog.Headline` has been renamed to `Dialog.Title`. Please update your code accordingly. + +- ``, ``, and `` have been introduced for better organization and flexibility. + +- The internal layout now uses grid areas, ensuring consistent ordering and layout of the dialog elements. + +- Existing implementations of the `` component will need to be updated to use these new subcomponents. + diff --git a/docs/content/components/overlay/dialog/dialog-dismiss.demo.tsx b/docs/content/components/overlay/dialog/dialog-dismiss.demo.tsx index b0f903fd9b..effca5a374 100644 --- a/docs/content/components/overlay/dialog/dialog-dismiss.demo.tsx +++ b/docs/content/components/overlay/dialog/dialog-dismiss.demo.tsx @@ -1,11 +1,13 @@ -import { Button, Dialog, Headline, Text } from '@marigold/components'; +import { Button, Dialog, Text } from '@marigold/components'; export default () => ( - Information! - This is a simple info Dialog. + Information! + + This is a simple info Dialog. + ); diff --git a/docs/content/components/overlay/dialog/dialog-form.demo.tsx b/docs/content/components/overlay/dialog/dialog-form.demo.tsx index cda1b90c62..1c815f694a 100644 --- a/docs/content/components/overlay/dialog/dialog-form.demo.tsx +++ b/docs/content/components/overlay/dialog/dialog-form.demo.tsx @@ -1,11 +1,4 @@ -import { - Button, - Dialog, - Headline, - Inline, - Stack, - TextField, -} from '@marigold/components'; +import { Button, Dialog, Stack, TextField } from '@marigold/components'; export default () => ( @@ -13,17 +6,19 @@ export default () => ( {({ close }) => ( <> - Please log into account - - - - - - - - + Please log into account + + + + + + + + + + )} diff --git a/docs/content/components/overlay/dialog/dialog.mdx b/docs/content/components/overlay/dialog/dialog.mdx index 5fa5c56aa6..a4ba2f5de7 100644 --- a/docs/content/components/overlay/dialog/dialog.mdx +++ b/docs/content/components/overlay/dialog/dialog.mdx @@ -1,15 +1,18 @@ --- title: Dialog caption: Component for displaying dialogs. +badge: updated --- -The `` is a overlay component. This means it displays a modal as a reaction to an event. It appears in the middle of the display and blurs out the underlay. +The `` is an overlay component. This means it displays a modal as a reaction to an event. It appears in the middle of the display and blurs out the underlay. -This component can be styled in different parts. It has as parts `closeButton` and `container`, which is the component itself, and these parts can be styled separately. +This component can be styled in different parts. It has parts like closeButton and container, which is the component itself, and these parts can be styled separately. -A special thing is also that you can have props on the `` and on the `` itself. The `closeButton` is an optionally property with which you can close the ``. +Additionally, the `` is composed of other flexible parts: ``, ``, ``, and `` holds the primary content or message, while the `` defines interactive buttons for user decisions. `` can be used to place additional information , ensuring clear separation and organization within the dialog. -The `` can also be used if you want to controll it for example within a ``. +A special feature is that you can also pass props to the `` and on the `` itself. The closeButton is an optional property that allows you to close the ``. + +The `` can also be controlled programmatically, for example, when used within a ``. ## Import diff --git a/packages/components/src/Dialog/Dialog.stories.tsx b/packages/components/src/Dialog/Dialog.stories.tsx index 5fb7c247f9..721a74d6b5 100644 --- a/packages/components/src/Dialog/Dialog.stories.tsx +++ b/packages/components/src/Dialog/Dialog.stories.tsx @@ -1,12 +1,8 @@ /* eslint-disable react-hooks/rules-of-hooks */ import type { Meta, StoryObj } from '@storybook/react'; import { useState } from 'react'; -import { Body } from '../Body'; import { Button } from '../Button'; import { Checkbox, CheckboxGroup } from '../Checkbox'; -import { Container } from '../Container'; -import { Footer } from '../Footer'; -import { Header } from '../Header'; import { Inline } from '../Inline'; import { Menu } from '../Menu'; import { Modal, ModalProps } from '../Overlay/Modal'; @@ -69,8 +65,8 @@ export const Basic: Story = { return ( - - This is a headline! + + This is a headline! This is some not so very long text. @@ -83,20 +79,20 @@ export const Form: Story = { return ( - + {({ close }) => ( <> - Please log into account - + Please log into account + - - - - - + + + + + )} @@ -109,9 +105,11 @@ export const CustomTitleProps: Story = { render: args => ( - - This is a headline! - This is some not so very long text. + + This is a headline! + + This is some not so very long text. + ), @@ -121,34 +119,30 @@ export const ScrollableContent: Story = { render: args => ( - - -
- This is a headline! -
- - - This is some not so very long text. -
- - One - Two - Three - Four - Five - Six - Seven - Eight - Nine - Ten - -
- - -
- -
- + + This is a headline! + + + This is some not so very long text. +
+ + One + Two + Three + Four + Five + Six + Seven + Eight + Nine + Ten + +
+ + + + + ), @@ -158,14 +152,12 @@ export const StickyFooter: Story = { render: args => ( - + + This is a headline!
-
- This is a headline! - This is some additional text that is always visible! -
+ This is some additional text that is always visible!
- + Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, @@ -181,11 +173,11 @@ export const StickyFooter: Story = { erat. Aliquam erat volutpat. Nam dui mi, tincidunt quis, accumsan porttitor, facilisis luctus, metus - +
-
+ -
+
@@ -221,14 +213,12 @@ export const WithDialogController: Story = { {({ close }) => ( - -
- Confirm delete -
- + <> + Confirm delete + Do you really wanna delete this? - -
+ + -
- + + )} diff --git a/packages/components/src/Dialog/Dialog.test.tsx b/packages/components/src/Dialog/Dialog.test.tsx index 17a66b5ddd..f238f1bdfe 100644 --- a/packages/components/src/Dialog/Dialog.test.tsx +++ b/packages/components/src/Dialog/Dialog.test.tsx @@ -33,6 +33,7 @@ const theme: Theme = { }), }, Headline: cva(''), + Header: cva(''), Underlay: cva('bg-black opacity-5'), }, }; @@ -55,7 +56,7 @@ test('renders children correctly', () => { - Headline + Headline Content @@ -91,7 +92,7 @@ test('dialog can be opened by button', () => { - Headline + Headline Content @@ -109,7 +110,7 @@ test('optionally renders a close button', () => { - Headline + Headline Content @@ -132,7 +133,7 @@ test('supoorts closing the dialog with escape key', async () => { - Headline + Headline Content @@ -152,7 +153,7 @@ test('close Dialog by clicking on the Underlay', () => { - Headline + Headline Content @@ -193,7 +194,7 @@ test('supports title for accessability reasons', () => { - Headline + Headline Content @@ -210,12 +211,68 @@ test('supports title for accessability reasons', () => { expect(headline.id).toBe(dialog.getAttribute('aria-labelledby')); }); +test('supports dialog contents', () => { + render( + + + + Headline + Content + + + ); + const button = screen.getByText('Open'); + fireEvent.click(button); + + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAttribute('aria-labelledby'); + const dialogContent = screen.getByText('Content'); + expect(dialogContent).toBeInTheDocument(); +}); + +test('supports dialog actions', () => { + render( + + + + Headline + + + + + + + ); + const button = screen.getByText('Open'); + fireEvent.click(button); + + const cancelButton = screen.getByText('Cancel'); + expect(cancelButton).toBeInTheDocument(); + + const loginButton = screen.getByText('Login'); + expect(loginButton).toBeInTheDocument(); +}); + +test('supports dialog footer', () => { + render( + + + Footer + + ); + const button = screen.getByText('Open'); + fireEvent.click(button); + + const footer = screen.getByText('Footer'); + expect(footer).toBeInTheDocument(); +}); + test('child function is passed an id for the dialog title (a11y)', () => { render( - Custom Headline + Custom Headline ); @@ -248,7 +305,7 @@ test('child function is passed an id for the dialog title (a11y)', () => { // expect(warn).toHaveBeenCalled(); // expect(warn.mock.calls[0][0]).toMatchInlineSnapshot( -// `"No child in found that can act as title for accessibility. Please add a
or as direct child."` +// `"No child in found that can act as title for accessibility. Please add a
or as direct child."` // ); // warn.mockRestore(); // }); @@ -259,7 +316,7 @@ test('supports focus and open dialog with keyboard', async () => { - Headline + Headline Content @@ -280,7 +337,7 @@ test('dialog has base classnames', () => { - Headline + Headline Content @@ -306,7 +363,7 @@ test('dialog has variant classnames', () => { - Headline + Headline Content @@ -323,7 +380,7 @@ test('dialog has variant classnames', () => { 'h-4 w-4 cursor-pointer border-none leading-normal outline-0 p-1 bg-black' ); expect(dialog.className).toMatchInlineSnapshot( - `"relative w-full outline-none p-5 bg-green-400"` + `"relative outline-none [&>*:not(:last-child)]:mb-4 grid [grid-template-areas:'title_button'_'content_content'_'actions_actions'_'footer_footer'] p-5 bg-green-400"` ); }); @@ -333,7 +390,7 @@ test('dialog supports size', () => { - Headline + Headline Content @@ -351,7 +408,7 @@ test('renders nothing by default', () => { - HeadlineContent + HeadlineContent ); @@ -370,7 +427,7 @@ test('dialog can be controlled', async () => { Open Dialog - Headline + Headline @@ -408,7 +465,7 @@ test('dialog can be controlled without a trigger', async () => { - Headline + Headline @@ -450,7 +507,7 @@ test('close state has a listener', async () => { {({ close }) => ( <> - Headline + Headline diff --git a/packages/components/src/Dialog/Dialog.tsx b/packages/components/src/Dialog/Dialog.tsx index 4362041d60..bc09fd76b0 100644 --- a/packages/components/src/Dialog/Dialog.tsx +++ b/packages/components/src/Dialog/Dialog.tsx @@ -1,7 +1,8 @@ -import { useContext } from 'react'; +import { ReactNode, useContext } from 'react'; import type RAC from 'react-aria-components'; import { Dialog, OverlayTriggerStateContext } from 'react-aria-components'; import { cn, useClassNames } from '@marigold/system'; +import { Header } from '../Header'; import { Headline, HeadlineProps } from '../Headline'; import { DialogTrigger } from './DialogTrigger'; @@ -14,7 +15,7 @@ interface CloseButtonProps { const CloseButton = ({ className }: CloseButtonProps) => { const { close } = useContext(OverlayTriggerStateContext); return ( -
+