From 545689e8fc2b1a00c2cc1989b25ac063b380d1de Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Mon, 27 Nov 2023 13:35:38 +0100 Subject: [PATCH 1/8] replacing old component with new DropdownMenu and solving issue --- .../DeletePopover/DeletePopover.module.css | 5 + .../DeletePopover/DeletePopover.tsx | 66 ++++++++ .../NavigationMenu/DeletePopover/index.ts | 1 + .../InputPopover/InputPopover.tsx | 158 ++++++++---------- .../NavigationMenu/NavigationMenu.tsx | 149 ++++++----------- 5 files changed, 197 insertions(+), 182 deletions(-) create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css new file mode 100644 index 00000000000..1bc2d4aac03 --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css @@ -0,0 +1,5 @@ +.buttonWrapper { + display: flex; + gap: 10px; + margin-top: 10px; +} diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx new file mode 100644 index 00000000000..099332c9fa4 --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx @@ -0,0 +1,66 @@ +import React, { ReactNode, useRef, useState } from 'react'; +import classes from './DeletePopover.module.css'; +import { Button, DropdownMenu, Paragraph, Popover } from '@digdir/design-system-react'; +import { useTranslation } from 'react-i18next'; +import { TrashIcon } from '@altinn/icons'; + +export type DeletePopoverProps = { + onClose: () => void; + onDelete: () => void; +}; + +/** + * @component + * Displays a dropdown menu item with a popover where the user can delete a layout + * + * @property {function}[onClose] - Function to be executed on close + * @property {function}[onDelete] - Function to be executed when clicking delete + * + * @returns {ReactNode} - The rendered component + */ +export const DeletePopover = ({ onClose, onDelete }: DeletePopoverProps): ReactNode => { + const { t } = useTranslation(); + + const newNameRef = useRef(null); + + const [popoverOpen, setPopoverOpen] = useState(false); + + const handleClose = () => { + onClose(); + setPopoverOpen((v) => !v); + }; + + return ( + <> + setPopoverOpen(true)} + id='delete-page-button' + color='danger' + ref={newNameRef} + aria-expanded={popoverOpen} + > + + {t('ux_editor.page_menu_delete')} + + + + {t('ux_editor.page_delete_text')} + {t('ux_editor.page_delete_information')} +
+ + +
+ + + + ); +}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts new file mode 100644 index 00000000000..70052c91f37 --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts @@ -0,0 +1 @@ +export { DeletePopover } from './DeletePopover'; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx index a27ed4b00e5..671f4a03e5e 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx @@ -1,83 +1,52 @@ -import React, { ReactNode, useRef, useEffect, ChangeEvent, useState } from 'react'; +import React, { ReactNode, ChangeEvent, useState, useRef } from 'react'; import classes from './InputPopover.module.css'; -import { Button, ErrorMessage, LegacyPopover, Textfield } from '@digdir/design-system-react'; +import { + Button, + DropdownMenu, + ErrorMessage, + Popover, + Textfield, +} from '@digdir/design-system-react'; import { useTranslation } from 'react-i18next'; import { getPageNameErrorKey } from '../../../../../utils/designViewUtils'; +import { PencilIcon } from '@altinn/icons'; export type InputPopoverProps = { - /** - * The old name of the page - */ + disabled: boolean; oldName: string; - /** - * The list containing all page names - */ layoutOrder: string[]; - /** - * Saves the new name of the page - * @param newName the new name to save - * @returns void - */ saveNewName: (newName: string) => void; - /** - * Function to be executed when closing the popover - * @param event optional mouse event - * @returns void - */ - onClose: (event?: React.MouseEvent | MouseEvent) => void; - /** - * If the popover is open or not - */ - open: boolean; - /** - * The component that triggers the opening of the popover - */ - trigger: ReactNode; + onClose: () => void; }; /** * @component - * Displays a popover where the user can edit the name of the page + * Displays a dropdown menu item with a popover where the user can edit the name of the page * + * @property {boolean}[disabled] - If the dropdown item is disabled * @property {string}[oldName] - The old name of the page * @property {string[]}[layoutOrder] - The list containing all page names * @property {function}[saveNewName] - Saves the new name of the page - * @property {function}[onClose] - Function to be executed when closing the popover - * @property {boolean}[open] - If the popover is open or not - * @property {ReactNode}[trigger] - The component that triggers the opening of the popover + * @property {function}[onClose] - Function to be executed on close * * @returns {ReactNode} - The rendered component */ export const InputPopover = ({ + disabled, oldName, layoutOrder, saveNewName, onClose, - open = false, - trigger, }: InputPopoverProps): ReactNode => { const { t } = useTranslation(); - const ref = useRef(null); + const newNameRef = useRef(null); + const [isEditDialogOpen, setIsEditDialogOpen] = useState(false); const [errorMessage, setErrorMessage] = useState(null); const [newName, setNewName] = useState(oldName); const shouldSavingBeEnabled = errorMessage === null && newName !== oldName; - useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - if (ref.current && !ref.current.contains(event.target)) { - onClose(event); - } - }; - if (open) { - document.addEventListener('mousedown', handleClickOutside); - } - return () => { - document.removeEventListener('mousedown', handleClickOutside); - }; - }, [onClose, open]); - /** * Handles the change of the new page name. If the name exists, is empty, is too * long, or has a wrong format, an error is set, otherwise the value displayed is changed. @@ -96,51 +65,68 @@ export const InputPopover = ({ const handleKeyPress = (event) => { if (event.key === 'Enter' && !errorMessage && oldName !== newName) { saveNewName(newName); - onClose(); + setIsEditDialogOpen(false); } else if (event.key === 'Escape') { - onClose(); + setIsEditDialogOpen(false); setNewName(oldName); setErrorMessage(null); } }; + const handleClose = () => { + onClose(); + setIsEditDialogOpen((v) => !v); + }; + return ( -
- - - - {errorMessage} - -
- - -
- -
+ onChange={handleOnChange} + value={newName} + error={errorMessage !== null} + /> + + {errorMessage} + +
+ + +
+ + + ); }; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx index 704cb4174a9..c924b087a52 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx @@ -1,14 +1,7 @@ -import React, { ReactNode, MouseEvent, SyntheticEvent, useState } from 'react'; +import React, { ReactNode, useState, useRef } from 'react'; import { useTranslation } from 'react-i18next'; -import { Button } from '@digdir/design-system-react'; -import { - MenuElipsisVerticalIcon, - ArrowUpIcon, - ArrowDownIcon, - PencilIcon, - TrashIcon, -} from '@navikt/aksel-icons'; -import { AltinnMenu, AltinnMenuItem } from 'app-shared/components'; +import { Button, DropdownMenu } from '@digdir/design-system-react'; +import { MenuElipsisVerticalIcon, ArrowUpIcon, ArrowDownIcon } from '@navikt/aksel-icons'; import { useFormLayoutSettingsQuery } from '../../../../hooks/queries/useFormLayoutSettingsQuery'; import { useUpdateLayoutOrderMutation } from '../../../../hooks/mutations/useUpdateLayoutOrderMutation'; import { useUpdateLayoutNameMutation } from '../../../../hooks/mutations/useUpdateLayoutNameMutation'; @@ -17,17 +10,14 @@ import { useSelector } from 'react-redux'; import { useDeleteLayoutMutation } from '../../../../hooks/mutations/useDeleteLayoutMutation'; import type { IAppState } from '../../../../types/global'; import { Divider } from 'app-shared/primitives'; -import { AltinnConfirmDialog } from 'app-shared/components'; import { useSearchParams } from 'react-router-dom'; import { firstAvailableLayout } from '../../../../utils/formLayoutsUtils'; import { InputPopover } from './InputPopover'; import { deepCopy } from 'app-shared/pure'; import { useAppContext } from '../../../../hooks/useAppContext'; +import { DeletePopover } from './DeletePopover'; export type NavigationMenuProps = { - /** - * The name of the page - */ pageName: string; pageIsReceipt: boolean; }; @@ -37,6 +27,7 @@ export type NavigationMenuProps = { * Displays the buttons to move a page accoridon up or down, edit the name and delete the page * * @property {string}[pageName] - The name of the page + * @property {boolean}[pageIsReceipt] - If the page is a receipt page * * @returns {ReactNode} - The rendered component */ @@ -61,29 +52,17 @@ export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps) const { mutate: deleteLayout } = useDeleteLayoutMutation(org, app, selectedLayoutSet); const { mutate: updateLayoutName } = useUpdateLayoutNameMutation(org, app, selectedLayoutSet); - const [menuAnchorEl, setMenuAnchorEl] = useState(null); - const [isConfirmDeleteDialogOpen, setIsConfirmDeleteDialogOpen] = useState(); - const [isEditDialogOpen, setIsEditDialogOpen] = useState(); - const [searchParams, setSearchParams] = useSearchParams(); const selectedLayout = searchParams.get('layout'); - const onPageSettingsClick = (event: MouseEvent) => - setMenuAnchorEl(event.currentTarget); - - const onMenuClose = (_event: SyntheticEvent) => setMenuAnchorEl(null); + const settingsRef = useRef(null); + const [dropdownOpen, setDropdownOpen] = useState(false); - const onMenuItemClick = (event: SyntheticEvent, action: 'up' | 'down' | 'edit' | 'delete') => { - if (action === 'delete') { - setIsConfirmDeleteDialogOpen((prevState) => !prevState); - } else if (action === 'edit') { - setIsEditDialogOpen((prevState) => !prevState); - } else { - if (action === 'up' || action === 'down') { - updateLayoutOrder({ layoutName: pageName, direction: action }); - } - setMenuAnchorEl(null); + const moveLayout = (action: 'up' | 'down') => { + if (action === 'up' || action === 'down') { + updateLayoutOrder({ layoutName: pageName, direction: action }); } + setDropdownOpen(false); }; const handleConfirmDelete = () => { @@ -104,76 +83,54 @@ export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps)
); }; From 7666a7bbdca937944d301264c6111641a3c5a7a5 Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Mon, 27 Nov 2023 14:15:55 +0100 Subject: [PATCH 2/8] adding tests --- .../DeletePopover/DeletePopover.test.tsx | 74 +++++++++++++++++++ .../DeletePopover/DeletePopover.tsx | 2 +- .../InputPopover/InputPopover.test.tsx | 61 ++++++++++----- .../InputPopover/InputPopover.tsx | 16 ---- .../NavigationMenu/NavigationMenu.test.tsx | 26 ++++--- 5 files changed, 136 insertions(+), 43 deletions(-) create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx new file mode 100644 index 00000000000..b8f134b5aca --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx @@ -0,0 +1,74 @@ +import React from 'react'; +import { render, act, screen } from '@testing-library/react'; +import { DeletePopover, DeletePopoverProps } from './DeletePopover'; +import userEvent from '@testing-library/user-event'; +import { textMock } from '../../../../../../../../testing/mocks/i18nMock'; + +const mockOnClose = jest.fn(); +const mockOnDelete = jest.fn(); + +const defaultProps: DeletePopoverProps = { + onClose: mockOnClose, + onDelete: mockOnDelete, +}; + +describe('DeletePopover', () => { + afterEach(jest.clearAllMocks); + + it('does hides dropdown menu item by default when not open', () => { + render(); + + const deleteButton = screen.queryByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButton).not.toBeInTheDocument(); + }); + + it('opens the popover when the dropdown menu item is clicked', async () => { + render(); + + const deleteButton = screen.queryByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButton).not.toBeInTheDocument(); + + await openDropdownMenuItem(); + + const deleteButtonAfter = screen.getByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButtonAfter).toBeInTheDocument(); + }); + + it('calls "onClose" when cancel button is clicked', async () => { + const user = userEvent.setup(); + render(); + await openDropdownMenuItem(); + + const cancelButton = screen.getByRole('button', { name: textMock('general.cancel') }); + await act(() => user.click(cancelButton)); + + expect(mockOnClose).toHaveBeenCalledTimes(1); + }); + + it('calls "onDelete" when delete button is clicked', async () => { + const user = userEvent.setup(); + render(); + await openDropdownMenuItem(); + + const deleteButton = screen.getByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + await act(() => user.click(deleteButton)); + + expect(mockOnDelete).toHaveBeenCalledTimes(1); + }); +}); + +const openDropdownMenuItem = async () => { + const user = userEvent.setup(); + const dropdownMenuItem = screen.getByRole('menuitem', { + name: textMock('ux_editor.page_menu_delete'), + }); + await act(() => user.click(dropdownMenuItem)); +}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx index 099332c9fa4..a88bbc179e8 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx @@ -55,7 +55,7 @@ export const DeletePopover = ({ onClose, onDelete }: DeletePopoverProps): ReactN - diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx index 6e4fcaa6e7c..668dc435632 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx @@ -17,20 +17,39 @@ const mockSaveNewName = jest.fn(); const mockOnClose = jest.fn(); const defaultProps: InputPopoverProps = { + disabled: false, oldName: mockOldName, layoutOrder: mockLayoutOrder, saveNewName: mockSaveNewName, onClose: mockOnClose, - open: true, - trigger: , }; describe('InputPopover', () => { - const user = userEvent.setup(); afterEach(jest.clearAllMocks); + it('does hides dropdown menu item by default when not open', () => { + render(); + + const input = screen.queryByLabelText(textMock('ux_editor.input_popover_label')); + expect(input).not.toBeInTheDocument(); + }); + + it('opens the popover when the dropdown menu item is clicked', async () => { + render(); + + const input = screen.queryByLabelText(textMock('ux_editor.input_popover_label')); + expect(input).not.toBeInTheDocument(); + + await openDropdownMenuItem(); + + const inputAfter = screen.getByLabelText(textMock('ux_editor.input_popover_label')); + expect(inputAfter).toBeInTheDocument(); + }); + it('calls the "saveNewName" function when the confirm button is clicked', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -50,7 +69,9 @@ describe('InputPopover', () => { }); it('does not call "saveNewName" when input is same as old value', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -59,22 +80,10 @@ describe('InputPopover', () => { expect(mockSaveNewName).toHaveBeenCalledTimes(0); }); - it('saves the new name on Enter key press', async () => { - render(); - - const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); - expect(input).toHaveValue(mockOldName); - - await act(() => user.type(input, mockNewValue)); - await act(() => user.keyboard('{Enter}')); - - expect(mockSaveNewName).toHaveBeenCalledTimes(1); - expect(mockSaveNewName).toHaveBeenCalledWith(mockNewName); - expect(mockOnClose).toHaveBeenCalledTimes(1); - }); - it('cancels the new name on Escape key press', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -87,7 +96,9 @@ describe('InputPopover', () => { }); it('displays error message if new name is not unique', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -102,7 +113,9 @@ describe('InputPopover', () => { }); it('displays error message if new name is empty', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -117,7 +130,9 @@ describe('InputPopover', () => { }); it('displays error message if new name is too long', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -133,7 +148,9 @@ describe('InputPopover', () => { }); it('displays error message if new name has illegal format', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(input).toHaveValue(mockOldName); @@ -149,7 +166,9 @@ describe('InputPopover', () => { }); it('closes the popover when cancel button is clicked', async () => { + const user = userEvent.setup(); render(); + await openDropdownMenuItem(); const button = screen.getByRole('button', { name: textMock('general.cancel') }); await act(() => user.click(button)); @@ -157,3 +176,11 @@ describe('InputPopover', () => { expect(mockOnClose).toHaveBeenCalledTimes(1); }); }); + +const openDropdownMenuItem = async () => { + const user = userEvent.setup(); + const dropdownMenuItem = screen.getByRole('menuitem', { + name: textMock('ux_editor.page_menu_edit'), + }); + await act(() => user.click(dropdownMenuItem)); +}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx index 671f4a03e5e..03a20db8281 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx @@ -58,21 +58,6 @@ export const InputPopover = ({ setNewName(newNameCandidate); }; - /** - * If there is no error and the name is changed, and enter is clicked, the new name is saved. - * When Escape is clicked, the popover closes. - */ - const handleKeyPress = (event) => { - if (event.key === 'Enter' && !errorMessage && oldName !== newName) { - saveNewName(newName); - setIsEditDialogOpen(false); - } else if (event.key === 'Escape') { - setIsEditDialogOpen(false); - setNewName(oldName); - setErrorMessage(null); - } - }; - const handleClose = () => { onClose(); setIsEditDialogOpen((v) => !v); @@ -107,7 +92,6 @@ export const InputPopover = ({ color='first' variant='primary' onClick={() => saveNewName(newName)} - onKeyDown={handleKeyPress} disabled={!shouldSavingBeEnabled} size='small' > diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx index 9dde9eb8486..11f5254bb6e 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx @@ -49,7 +49,9 @@ describe('NavigationMenu', () => { const menuButton = screen.getByRole('button', { name: textMock('general.options') }); await act(() => user.click(menuButton)); - const elementInMenuAfter = screen.getByText(textMock('ux_editor.page_menu_up')); + const elementInMenuAfter = screen.getByRole('menuitem', { + name: textMock('ux_editor.page_menu_up'), + }); expect(elementInMenuAfter).toBeInTheDocument(); }); @@ -60,7 +62,9 @@ describe('NavigationMenu', () => { const menuButton = screen.getByRole('button', { name: textMock('general.options') }); await act(() => user.click(menuButton)); - const deleteButton = screen.getByText(textMock('ux_editor.page_menu_delete')); + const deleteButton = screen.getByRole('menuitem', { + name: textMock('ux_editor.page_menu_delete'), + }); await act(() => user.click(deleteButton)); const confirmButton = screen.getByRole('button', { @@ -74,8 +78,6 @@ describe('NavigationMenu', () => { mockPageName1, mockSelectedLayoutSet, ); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(mockSetSearchParams).toHaveBeenCalledWith({ layout: mockPageName2 }); }); @@ -84,7 +86,9 @@ describe('NavigationMenu', () => { await render(); await act(() => user.click(screen.getByTitle(textMock('general.options')))); - await act(() => user.click(screen.getByText(textMock('ux_editor.page_menu_edit')))); + await act(() => + user.click(screen.getByRole('menuitem', { name: textMock('ux_editor.page_menu_edit') })), + ); const inputField = screen.getByLabelText(textMock('ux_editor.input_popover_label')); expect(inputField).toHaveValue(mockPageName1); @@ -114,8 +118,10 @@ describe('NavigationMenu', () => { await act(() => user.click(screen.getByTitle(textMock('general.options')))); - const upButton = screen.queryByText(textMock('ux_editor.page_menu_up')); - const downButton = screen.queryByText(textMock('ux_editor.page_menu_down')); + const upButton = screen.queryByRole('menuitem', { name: textMock('ux_editor.page_menu_up') }); + const downButton = screen.queryByRole('menuitem', { + name: textMock('ux_editor.page_menu_down'), + }); expect(upButton).not.toBeInTheDocument(); expect(downButton).not.toBeInTheDocument(); @@ -127,8 +133,10 @@ describe('NavigationMenu', () => { await act(() => user.click(screen.getByTitle(textMock('general.options')))); - const upButton = screen.getByText(textMock('ux_editor.page_menu_up')); - const downButton = screen.getByText(textMock('ux_editor.page_menu_down')); + const upButton = screen.getByRole('menuitem', { name: textMock('ux_editor.page_menu_up') }); + const downButton = screen.getByRole('menuitem', { + name: textMock('ux_editor.page_menu_down'), + }); expect(upButton).toBeInTheDocument(); expect(downButton).toBeInTheDocument(); From 936c8fa5b768943b8d5c7f071387ea1cca34e38f Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Mon, 4 Dec 2023 14:35:54 +0100 Subject: [PATCH 3/8] refactor to use popover correctly --- .../DeletePopover/DeletePopover.module.css | 3 + .../DeletePopover/DeletePopover.test.tsx | 122 ++++++++++++++++++ .../DeletePopover/DeletePopover.tsx | 76 +++++++++++ .../DeletePopover/index.ts | 0 .../DeletePopover/DeletePopover.module.css | 5 - .../DeletePopover/DeletePopover.test.tsx | 74 ----------- .../DeletePopover/DeletePopover.tsx | 66 ---------- .../NavigationMenu/NavigationMenu.test.tsx | 28 ---- .../NavigationMenu/NavigationMenu.tsx | 25 +--- .../PageAccordion/PageAccordion.tsx | 2 + 10 files changed, 206 insertions(+), 195 deletions(-) create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx create mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx rename frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/{NavigationMenu => }/DeletePopover/index.ts (100%) delete mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css delete mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx delete mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css new file mode 100644 index 00000000000..fdc53556378 --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css @@ -0,0 +1,3 @@ +.popoverContent { + padding-bottom: 1rem; +} diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx new file mode 100644 index 00000000000..ec0458ab696 --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx @@ -0,0 +1,122 @@ +import React from 'react'; +import { act, screen, waitFor } from '@testing-library/react'; +import { DeletePopover, DeletePopoverProps } from './DeletePopover'; +import userEvent from '@testing-library/user-event'; +import { textMock } from '../../../../../../../testing/mocks/i18nMock'; +import { + queriesMock, + renderHookWithMockStore, + renderWithMockStore, +} from '../../../../testing/mocks'; +import { useFormLayoutSettingsQuery } from '../../../../hooks/queries/useFormLayoutSettingsQuery'; +import { formDesignerMock } from '../../../../testing/stateMocks'; +import { layout1NameMock, layout2NameMock } from '../../../../testing/layoutMock'; +import { ServicesContextProps } from 'app-shared/contexts/ServicesContext'; + +const deleteFormLayout = jest.fn().mockImplementation(() => Promise.resolve({})); + +const mockOrg = 'org'; +const mockApp = 'app'; +const mockPageName1: string = formDesignerMock.layout.selectedLayout; +const mockSelectedLayoutSet = 'test-layout-set'; +const mockPageName2 = layout2NameMock; + +const mockSetSearchParams = jest.fn(); +const mockSearchParams = { layout: mockPageName1 }; +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: () => ({ + org: mockOrg, + app: mockApp, + }), + useSearchParams: () => { + return [new URLSearchParams(mockSearchParams), mockSetSearchParams]; + }, +})); + +const defaultProps: DeletePopoverProps = { + pageName: layout1NameMock, +}; + +describe('DeleteModal', () => { + afterEach(jest.clearAllMocks); + + it('does not show the modal by default when not open', async () => { + await render(); + + const deleteButton = screen.queryByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButton).not.toBeInTheDocument(); + }); + + it('opens the modal when the button is clicked', async () => { + await render(); + + const deleteButton = screen.queryByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButton).not.toBeInTheDocument(); + + await openDeletePopover(); + + const deleteButtonAfter = screen.getByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + expect(deleteButtonAfter).toBeInTheDocument(); + }); + + it('calls "onDelete" when delete button is clicked', async () => { + const user = userEvent.setup(); + await render(); + await openDeletePopover(); + + const deleteButton = screen.getByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + await act(() => user.click(deleteButton)); + + expect(deleteFormLayout).toHaveBeenCalledTimes(1); + }); + + it('should update the url to new page when deleting selected page', async () => { + const user = userEvent.setup(); + await render(); + await openDeletePopover(); + + const deleteButton = screen.getByRole('button', { + name: textMock('ux_editor.page_delete_confirm'), + }); + await act(() => user.click(deleteButton)); + + expect(deleteFormLayout).toHaveBeenCalledTimes(1); + expect(mockSetSearchParams).toHaveBeenCalledWith({ layout: mockPageName2 }); + }); +}); + +const openDeletePopover = async () => { + const user = userEvent.setup(); + const dropdownMenuItem = screen.getByRole('button', { + name: textMock('general.delete'), + }); + await act(() => user.click(dropdownMenuItem)); +}; + +const waitForData = async () => { + const settingsResult = renderHookWithMockStore()(() => + useFormLayoutSettingsQuery(mockOrg, mockApp, mockSelectedLayoutSet), + ).renderHookResult.result; + + await waitFor(() => expect(settingsResult.current.isSuccess).toBe(true)); +}; + +const render = async (props: Partial = {}) => { + await waitForData(); + + const allQueries: ServicesContextProps = { + ...queriesMock, + deleteFormLayout, + }; + + return renderWithMockStore({}, allQueries)(); +}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx new file mode 100644 index 00000000000..db121a7deba --- /dev/null +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx @@ -0,0 +1,76 @@ +import React, { useState } from 'react'; +import classes from './DeletePopover.module.css'; +import { Button, Paragraph } from '@digdir/design-system-react'; +import { useTranslation } from 'react-i18next'; +import { TrashIcon } from '@altinn/icons'; +import { useSearchParams } from 'react-router-dom'; +import { useDeleteLayoutMutation } from '../../../../hooks/mutations/useDeleteLayoutMutation'; +import { useStudioUrlParams } from 'app-shared/hooks/useStudioUrlParams'; +import { useAppContext } from '../../../../hooks/useAppContext'; +import { firstAvailableLayout } from '../../../../utils/formLayoutsUtils'; +import { useFormLayoutSettingsQuery } from '../../../../hooks/queries/useFormLayoutSettingsQuery'; +import { AltinnConfirmDialog } from 'app-shared/components'; + +export type DeletePopoverProps = { + pageName: string; +}; + +/** + * @component + * Displays a modal with the option to delete a page + * + * @property {string}[pageName] - The name of the page to delete + * + * @returns {JSX.Element} - The rendered component + */ +export const DeletePopover = ({ pageName }: DeletePopoverProps): JSX.Element => { + const { org, app } = useStudioUrlParams(); + const { selectedLayoutSet } = useAppContext(); + const { t } = useTranslation(); + + const { mutate: deleteLayout } = useDeleteLayoutMutation(org, app, selectedLayoutSet); + const [searchParams, setSearchParams] = useSearchParams(); + const selectedLayout = searchParams.get('layout'); + const { data: formLayoutSettings } = useFormLayoutSettingsQuery(org, app, selectedLayoutSet); + const layoutOrder = formLayoutSettings?.pages.order; + + const [isOpen, setIsOpen] = useState(false); + + const handleConfirmDelete = () => { + deleteLayout(pageName); + + if (selectedLayout === pageName) { + const layoutToSelect = firstAvailableLayout(pageName, layoutOrder); + setSearchParams({ layout: layoutToSelect }); + } + }; + + const handleClose = () => setIsOpen((prevState) => !prevState); + + return ( + } + onClick={(event: React.MouseEvent) => { + event.stopPropagation(); + setIsOpen((prevState) => !prevState); + }} + title={t('general.delete')} + variant='tertiary' + size='small' + /> + } + > +
+ {t('ux_editor.page_delete_text')} + {t('ux_editor.page_delete_information')} +
+ + ); +}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/index.ts similarity index 100% rename from frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/index.ts rename to frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/index.ts diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css deleted file mode 100644 index 1bc2d4aac03..00000000000 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.module.css +++ /dev/null @@ -1,5 +0,0 @@ -.buttonWrapper { - display: flex; - gap: 10px; - margin-top: 10px; -} diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx deleted file mode 100644 index b8f134b5aca..00000000000 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.test.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import React from 'react'; -import { render, act, screen } from '@testing-library/react'; -import { DeletePopover, DeletePopoverProps } from './DeletePopover'; -import userEvent from '@testing-library/user-event'; -import { textMock } from '../../../../../../../../testing/mocks/i18nMock'; - -const mockOnClose = jest.fn(); -const mockOnDelete = jest.fn(); - -const defaultProps: DeletePopoverProps = { - onClose: mockOnClose, - onDelete: mockOnDelete, -}; - -describe('DeletePopover', () => { - afterEach(jest.clearAllMocks); - - it('does hides dropdown menu item by default when not open', () => { - render(); - - const deleteButton = screen.queryByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButton).not.toBeInTheDocument(); - }); - - it('opens the popover when the dropdown menu item is clicked', async () => { - render(); - - const deleteButton = screen.queryByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButton).not.toBeInTheDocument(); - - await openDropdownMenuItem(); - - const deleteButtonAfter = screen.getByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButtonAfter).toBeInTheDocument(); - }); - - it('calls "onClose" when cancel button is clicked', async () => { - const user = userEvent.setup(); - render(); - await openDropdownMenuItem(); - - const cancelButton = screen.getByRole('button', { name: textMock('general.cancel') }); - await act(() => user.click(cancelButton)); - - expect(mockOnClose).toHaveBeenCalledTimes(1); - }); - - it('calls "onDelete" when delete button is clicked', async () => { - const user = userEvent.setup(); - render(); - await openDropdownMenuItem(); - - const deleteButton = screen.getByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - await act(() => user.click(deleteButton)); - - expect(mockOnDelete).toHaveBeenCalledTimes(1); - }); -}); - -const openDropdownMenuItem = async () => { - const user = userEvent.setup(); - const dropdownMenuItem = screen.getByRole('menuitem', { - name: textMock('ux_editor.page_menu_delete'), - }); - await act(() => user.click(dropdownMenuItem)); -}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx deleted file mode 100644 index a88bbc179e8..00000000000 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/DeletePopover/DeletePopover.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import React, { ReactNode, useRef, useState } from 'react'; -import classes from './DeletePopover.module.css'; -import { Button, DropdownMenu, Paragraph, Popover } from '@digdir/design-system-react'; -import { useTranslation } from 'react-i18next'; -import { TrashIcon } from '@altinn/icons'; - -export type DeletePopoverProps = { - onClose: () => void; - onDelete: () => void; -}; - -/** - * @component - * Displays a dropdown menu item with a popover where the user can delete a layout - * - * @property {function}[onClose] - Function to be executed on close - * @property {function}[onDelete] - Function to be executed when clicking delete - * - * @returns {ReactNode} - The rendered component - */ -export const DeletePopover = ({ onClose, onDelete }: DeletePopoverProps): ReactNode => { - const { t } = useTranslation(); - - const newNameRef = useRef(null); - - const [popoverOpen, setPopoverOpen] = useState(false); - - const handleClose = () => { - onClose(); - setPopoverOpen((v) => !v); - }; - - return ( - <> - setPopoverOpen(true)} - id='delete-page-button' - color='danger' - ref={newNameRef} - aria-expanded={popoverOpen} - > - - {t('ux_editor.page_menu_delete')} - - - - {t('ux_editor.page_delete_text')} - {t('ux_editor.page_delete_information')} -
- - -
- - - - ); -}; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx index 11f5254bb6e..81b7aa385a2 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.test.tsx @@ -9,14 +9,12 @@ import { renderWithMockStore, } from '../../../../testing/mocks'; import { formDesignerMock } from '../../../../testing/stateMocks'; -import { layout2NameMock } from '../../../../testing/layoutMock'; import { useFormLayoutSettingsQuery } from '../../../../hooks/queries/useFormLayoutSettingsQuery'; const mockOrg = 'org'; const mockApp = 'app'; const mockPageName1: string = formDesignerMock.layout.selectedLayout; const mockSelectedLayoutSet = 'test-layout-set'; -const mockPageName2 = layout2NameMock; const mockSetSearchParams = jest.fn(); const mockSearchParams = { layout: mockPageName1 }; @@ -55,32 +53,6 @@ describe('NavigationMenu', () => { expect(elementInMenuAfter).toBeInTheDocument(); }); - it('should update the url to new page when deleting selected page', async () => { - const user = userEvent.setup(); - await render(); - - const menuButton = screen.getByRole('button', { name: textMock('general.options') }); - await act(() => user.click(menuButton)); - - const deleteButton = screen.getByRole('menuitem', { - name: textMock('ux_editor.page_menu_delete'), - }); - await act(() => user.click(deleteButton)); - - const confirmButton = screen.getByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - await act(() => user.click(confirmButton)); - - expect(queriesMock.deleteFormLayout).toBeCalledWith( - mockOrg, - mockApp, - mockPageName1, - mockSelectedLayoutSet, - ); - expect(mockSetSearchParams).toHaveBeenCalledWith({ layout: mockPageName2 }); - }); - it('Calls updateFormLayoutName with new name when name is changed by the user', async () => { const user = userEvent.setup(); await render(); diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx index c924b087a52..fc771dec222 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/NavigationMenu.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode, useState, useRef } from 'react'; +import React, { useState, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { Button, DropdownMenu } from '@digdir/design-system-react'; import { MenuElipsisVerticalIcon, ArrowUpIcon, ArrowDownIcon } from '@navikt/aksel-icons'; @@ -7,15 +7,11 @@ import { useUpdateLayoutOrderMutation } from '../../../../hooks/mutations/useUpd import { useUpdateLayoutNameMutation } from '../../../../hooks/mutations/useUpdateLayoutNameMutation'; import { useStudioUrlParams } from 'app-shared/hooks/useStudioUrlParams'; import { useSelector } from 'react-redux'; -import { useDeleteLayoutMutation } from '../../../../hooks/mutations/useDeleteLayoutMutation'; import type { IAppState } from '../../../../types/global'; -import { Divider } from 'app-shared/primitives'; import { useSearchParams } from 'react-router-dom'; -import { firstAvailableLayout } from '../../../../utils/formLayoutsUtils'; import { InputPopover } from './InputPopover'; import { deepCopy } from 'app-shared/pure'; import { useAppContext } from '../../../../hooks/useAppContext'; -import { DeletePopover } from './DeletePopover'; export type NavigationMenuProps = { pageName: string; @@ -29,9 +25,9 @@ export type NavigationMenuProps = { * @property {string}[pageName] - The name of the page * @property {boolean}[pageIsReceipt] - If the page is a receipt page * - * @returns {ReactNode} - The rendered component + * @returns {JSX.Element} - The rendered component */ -export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps): ReactNode => { +export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps): JSX.Element => { const { t } = useTranslation(); const { org, app } = useStudioUrlParams(); @@ -49,11 +45,9 @@ export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps) const disableDown = layoutOrder.indexOf(pageName) === layoutOrder.length - 1; const { mutate: updateLayoutOrder } = useUpdateLayoutOrderMutation(org, app, selectedLayoutSet); - const { mutate: deleteLayout } = useDeleteLayoutMutation(org, app, selectedLayoutSet); const { mutate: updateLayoutName } = useUpdateLayoutNameMutation(org, app, selectedLayoutSet); const [searchParams, setSearchParams] = useSearchParams(); - const selectedLayout = searchParams.get('layout'); const settingsRef = useRef(null); const [dropdownOpen, setDropdownOpen] = useState(false); @@ -65,15 +59,6 @@ export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps) setDropdownOpen(false); }; - const handleConfirmDelete = () => { - deleteLayout(pageName); - - if (selectedLayout === pageName) { - const layoutToSelect = firstAvailableLayout(pageName, layoutOrder); - setSearchParams({ layout: layoutToSelect }); - } - }; - const handleSaveNewName = (newName: string) => { updateLayoutName({ oldName: pageName, newName }); setSearchParams({ ...deepCopy(searchParams), layout: newName }); @@ -126,10 +111,6 @@ export const NavigationMenu = ({ pageName, pageIsReceipt }: NavigationMenuProps) onClose={() => setDropdownOpen(false)} /> - - - setDropdownOpen(false)} onDelete={handleConfirmDelete} /> - ); diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx index 18ea10f42f0..1a496d2e8b7 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx @@ -4,6 +4,7 @@ import cn from 'classnames'; import { Accordion } from '@digdir/design-system-react'; import { NavigationMenu } from './NavigationMenu'; import * as testids from '../../../../../../testing/testids'; +import { DeletePopover } from './DeletePopover'; export type PageAccordionProps = { pageName: string; @@ -44,6 +45,7 @@ export const PageAccordion = ({
+
Date: Wed, 6 Dec 2023 15:44:50 +0100 Subject: [PATCH 4/8] fixing broken code --- .../DesignView/PageAccordion/DeletePopover/DeletePopover.tsx | 2 +- .../PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx index db121a7deba..af5b697bbb5 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import classes from './DeletePopover.module.css'; import { Button, Paragraph } from '@digdir/design-system-react'; import { useTranslation } from 'react-i18next'; -import { TrashIcon } from '@altinn/icons'; +import { TrashIcon } from '@studio/icons'; import { useSearchParams } from 'react-router-dom'; import { useDeleteLayoutMutation } from '../../../../hooks/mutations/useDeleteLayoutMutation'; import { useStudioUrlParams } from 'app-shared/hooks/useStudioUrlParams'; diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx index 03a20db8281..045189e4b51 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx @@ -9,7 +9,7 @@ import { } from '@digdir/design-system-react'; import { useTranslation } from 'react-i18next'; import { getPageNameErrorKey } from '../../../../../utils/designViewUtils'; -import { PencilIcon } from '@altinn/icons'; +import { PencilIcon } from '@studio/icons'; export type InputPopoverProps = { disabled: boolean; From 7e62993fd7cb388310b0a480080f8d4bcd4a357d Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Thu, 7 Dec 2023 11:48:00 +0100 Subject: [PATCH 5/8] trying to fix broken tests --- frontend/language/src/nb.json | 1 - .../DeletePopover/DeletePopover.module.css | 3 - .../DeletePopover/DeletePopover.test.tsx | 41 ++++---------- .../DeletePopover/DeletePopover.tsx | 56 ++++++------------- 4 files changed, 28 insertions(+), 73 deletions(-) delete mode 100644 frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css diff --git a/frontend/language/src/nb.json b/frontend/language/src/nb.json index 5787505a057..86469d395ba 100644 --- a/frontend/language/src/nb.json +++ b/frontend/language/src/nb.json @@ -1536,7 +1536,6 @@ "ux_editor.page": "Side", "ux_editor.page_delete_confirm": "Ja, slett skjemasiden", "ux_editor.page_delete_information": "Alt innholdet på siden vil bli fjernet.", - "ux_editor.page_delete_text": "Er du sikker på at du vil slette denne siden?", "ux_editor.page_menu_delete": "Slett", "ux_editor.page_menu_down": "Flytt ned", "ux_editor.page_menu_edit": "Gi nytt navn", diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css deleted file mode 100644 index fdc53556378..00000000000 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.module.css +++ /dev/null @@ -1,3 +0,0 @@ -.popoverContent { - padding-bottom: 1rem; -} diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx index ec0458ab696..f017699329a 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.test.tsx @@ -41,42 +41,21 @@ const defaultProps: DeletePopoverProps = { describe('DeleteModal', () => { afterEach(jest.clearAllMocks); - it('does not show the modal by default when not open', async () => { + it.only('Calls deleteLayout with pageName when delete button is clicked and deletion is confirmed', async () => { + jest.spyOn(window, 'confirm').mockImplementation(jest.fn(() => true)); await render(); - - const deleteButton = screen.queryByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButton).not.toBeInTheDocument(); - }); - - it('opens the modal when the button is clicked', async () => { - await render(); - - const deleteButton = screen.queryByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButton).not.toBeInTheDocument(); - await openDeletePopover(); - const deleteButtonAfter = screen.getByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - expect(deleteButtonAfter).toBeInTheDocument(); + await screen.getByRole('button', { name: textMock('general.delete') }).click(); + expect(deleteFormLayout).toHaveBeenCalledTimes(1); + expect(deleteFormLayout).toHaveBeenCalledWith(mockPageName1); }); - it('calls "onDelete" when delete button is clicked', async () => { - const user = userEvent.setup(); - await render(); - await openDeletePopover(); - - const deleteButton = screen.getByRole('button', { - name: textMock('ux_editor.page_delete_confirm'), - }); - await act(() => user.click(deleteButton)); - - expect(deleteFormLayout).toHaveBeenCalledTimes(1); + it('Does not call deleteLayout when delete button is clicked, but deletion is not confirmed', async () => { + jest.spyOn(window, 'confirm').mockImplementation(jest.fn(() => false)); + render(); + await screen.getByRole('button', { name: textMock('general.delete') }).click(); + expect(deleteFormLayout).not.toHaveBeenCalled(); }); it('should update the url to new page when deleting selected page', async () => { diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx index af5b697bbb5..2cdcd9b5364 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/DeletePopover/DeletePopover.tsx @@ -1,6 +1,5 @@ -import React, { useState } from 'react'; -import classes from './DeletePopover.module.css'; -import { Button, Paragraph } from '@digdir/design-system-react'; +import React, { useCallback } from 'react'; +import { Button } from '@digdir/design-system-react'; import { useTranslation } from 'react-i18next'; import { TrashIcon } from '@studio/icons'; import { useSearchParams } from 'react-router-dom'; @@ -9,7 +8,6 @@ import { useStudioUrlParams } from 'app-shared/hooks/useStudioUrlParams'; import { useAppContext } from '../../../../hooks/useAppContext'; import { firstAvailableLayout } from '../../../../utils/formLayoutsUtils'; import { useFormLayoutSettingsQuery } from '../../../../hooks/queries/useFormLayoutSettingsQuery'; -import { AltinnConfirmDialog } from 'app-shared/components'; export type DeletePopoverProps = { pageName: string; @@ -34,43 +32,25 @@ export const DeletePopover = ({ pageName }: DeletePopoverProps): JSX.Element => const { data: formLayoutSettings } = useFormLayoutSettingsQuery(org, app, selectedLayoutSet); const layoutOrder = formLayoutSettings?.pages.order; - const [isOpen, setIsOpen] = useState(false); + const handleConfirmDelete = useCallback(() => { + if (confirm(t('ux_editor.page_delete_text'))) { + deleteLayout(pageName); - const handleConfirmDelete = () => { - deleteLayout(pageName); - - if (selectedLayout === pageName) { - const layoutToSelect = firstAvailableLayout(pageName, layoutOrder); - setSearchParams({ layout: layoutToSelect }); + if (selectedLayout === pageName) { + const layoutToSelect = firstAvailableLayout(pageName, layoutOrder); + setSearchParams({ layout: layoutToSelect }); + } } - }; - - const handleClose = () => setIsOpen((prevState) => !prevState); + }, [deleteLayout, layoutOrder, pageName, selectedLayout, setSearchParams, t]); return ( - } - onClick={(event: React.MouseEvent) => { - event.stopPropagation(); - setIsOpen((prevState) => !prevState); - }} - title={t('general.delete')} - variant='tertiary' - size='small' - /> - } - > -
- {t('ux_editor.page_delete_text')} - {t('ux_editor.page_delete_information')} -
- + @@ -33,7 +40,7 @@ const mockChildren: ReactNode = ( const mockOnClick = jest.fn(); const defaultProps: PageAccordionProps = { - pageName: mockPageName, + pageName: mockPageName1, children: mockChildren, isOpen: false, onClick: mockOnClick, @@ -46,7 +53,7 @@ describe('PageAccordion', () => { const user = userEvent.setup(); await render(); - const accordionButton = screen.getByRole('button', { name: mockPageName }); + const accordionButton = screen.getByRole('button', { name: mockPageName1 }); await act(() => user.click(accordionButton)); expect(mockOnClick).toHaveBeenCalledTimes(1); @@ -65,6 +72,23 @@ describe('PageAccordion', () => { const elementInMenuAfter = screen.getByText(textMock('ux_editor.page_menu_up')); expect(elementInMenuAfter).toBeInTheDocument(); }); + + it('Calls deleteLayout with pageName when delete button is clicked and deletion is confirmed, and updates the url correctly', async () => { + jest.spyOn(window, 'confirm').mockImplementation(jest.fn(() => true)); + await render(); + + await screen.getByRole('button', { name: textMock('general.delete') }).click(); + expect(mockDeleteFormLayout).toHaveBeenCalledTimes(1); + expect(mockDeleteFormLayout).toHaveBeenCalledWith(mockPageName1); + expect(mockSetSearchParams).toHaveBeenCalledWith({ layout: mockPageName2 }); + }); + + it('Does not call deleteLayout when delete button is clicked, but deletion is not confirmed', async () => { + jest.spyOn(window, 'confirm').mockImplementation(jest.fn(() => false)); + await render(); + await screen.getByRole('button', { name: textMock('general.delete') }).click(); + expect(mockDeleteFormLayout).not.toHaveBeenCalled(); + }); }); const waitForData = async () => { diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx index 1a496d2e8b7..859ab542ec0 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx @@ -1,10 +1,17 @@ -import React, { ReactNode } from 'react'; +import React, { ReactNode, useCallback } from 'react'; import classes from './PageAccordion.module.css'; import cn from 'classnames'; -import { Accordion } from '@digdir/design-system-react'; +import { Accordion, Button } from '@digdir/design-system-react'; import { NavigationMenu } from './NavigationMenu'; import * as testids from '../../../../../../testing/testids'; -import { DeletePopover } from './DeletePopover'; +import { TrashIcon } from '@studio/icons'; +import { useTranslation } from 'react-i18next'; +import { useSearchParams } from 'react-router-dom'; +import { useStudioUrlParams } from 'app-shared/hooks/useStudioUrlParams'; +import { useAppContext } from '../../../hooks/useAppContext'; +import { firstAvailableLayout } from '../../../utils/formLayoutsUtils'; +import { useFormLayoutSettingsQuery } from '../../../hooks/queries/useFormLayoutSettingsQuery'; +import { useDeleteLayout } from './useDeleteLayout'; export type PageAccordionProps = { pageName: string; @@ -34,6 +41,28 @@ export const PageAccordion = ({ onClick, pageIsReceipt, }: PageAccordionProps): ReactNode => { + const { t } = useTranslation(); + const { org, app } = useStudioUrlParams(); + const { selectedLayoutSet } = useAppContext(); + const [searchParams, setSearchParams] = useSearchParams(); + const selectedLayout = searchParams.get('layout'); + + const { data: formLayoutSettings } = useFormLayoutSettingsQuery(org, app, selectedLayoutSet); + const layoutOrder = formLayoutSettings?.pages.order; + + const deleteLayout = useDeleteLayout(); + + const handleConfirmDelete = useCallback(() => { + if (confirm(t('ux_editor.page_delete_text'))) { + deleteLayout(pageName); + + if (selectedLayout === pageName) { + const layoutToSelect = firstAvailableLayout(pageName, layoutOrder); + setSearchParams({ layout: layoutToSelect }); + } + } + }, [deleteLayout, layoutOrder, pageName, selectedLayout, setSearchParams, t]); + return (
- +
{ + const { org, app } = useStudioUrlParams(); + const { selectedLayoutSet } = useAppContext(); + const { mutate: deleteLayout } = useDeleteLayoutMutation(org, app, selectedLayoutSet); + return useMemo(() => deleteLayout, [deleteLayout]); +}; From 421758c73f82dc9ecb64f6d4d9a07852d3d1bbff Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Mon, 11 Dec 2023 10:41:35 +0100 Subject: [PATCH 7/8] fixing feedback from PR --- .../InputPopover/InputPopover.test.tsx | 17 +++++++++++++++++ .../InputPopover/InputPopover.tsx | 12 ++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx index 668dc435632..06040710286 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx @@ -46,6 +46,23 @@ describe('InputPopover', () => { expect(inputAfter).toBeInTheDocument(); }); + it('saves the new name on Enter key press', async () => { + const user = userEvent.setup(); + render(); + + await openDropdownMenuItem(); + + const input = screen.getByLabelText(textMock('ux_editor.input_popover_label')); + expect(input).toHaveValue(mockOldName); + + await act(() => user.type(input, mockNewValue)); + await act(() => user.keyboard('{Enter}')); + + expect(mockSaveNewName).toHaveBeenCalledTimes(1); + expect(mockSaveNewName).toHaveBeenCalledWith(mockNewName); + expect(mockOnClose).toHaveBeenCalledTimes(1); + }); + it('calls the "saveNewName" function when the confirm button is clicked', async () => { const user = userEvent.setup(); render(); diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx index 045189e4b51..0b6d6c8dd74 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode, ChangeEvent, useState, useRef } from 'react'; +import React, { ReactNode, ChangeEvent, useState, useRef, KeyboardEvent } from 'react'; import classes from './InputPopover.module.css'; import { Button, @@ -58,9 +58,16 @@ export const InputPopover = ({ setNewName(newNameCandidate); }; + const handleKeyPress = (event: KeyboardEvent) => { + if (event.key === 'Enter' && !errorMessage && oldName !== newName) { + saveNewName(newName); + onClose(); + } + }; + const handleClose = () => { onClose(); - setIsEditDialogOpen((v) => !v); + setIsEditDialogOpen(false); }; return ( @@ -81,6 +88,7 @@ export const InputPopover = ({ label={t('ux_editor.input_popover_label')} size='small' onChange={handleOnChange} + onKeyDown={handleKeyPress} value={newName} error={errorMessage !== null} /> From e278956bb5ae833f19d50c5bd60e4b695b892667 Mon Sep 17 00:00:00 2001 From: WilliamThorenfeldt Date: Mon, 11 Dec 2023 13:12:29 +0100 Subject: [PATCH 8/8] fixing comment --- .../PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx index 0b6d6c8dd74..bb4bc97af29 100644 --- a/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx +++ b/frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.tsx @@ -61,7 +61,7 @@ export const InputPopover = ({ const handleKeyPress = (event: KeyboardEvent) => { if (event.key === 'Enter' && !errorMessage && oldName !== newName) { saveNewName(newName); - onClose(); + handleClose(); } };