Skip to content

Commit

Permalink
fix(pn-9620): fix spacing in modal PF/PA/PG for tablet (#1166)
Browse files Browse the repository at this point in the history
  • Loading branch information
SarahDonvito authored Mar 28, 2024
1 parent 2e0e3db commit 2eda2f6
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ vi.mock('../../../hooks', () => ({
describe('ApiErrorWrapper', () => {
const original = window.location;
const reloadText = 'Ricarica';
const user = userEvent.setup();

beforeAll(() => {
Object.defineProperty(window, 'location', {
Expand Down Expand Up @@ -67,7 +68,7 @@ describe('ApiErrorWrapper', () => {

const reloadItemComponent = screen.getByText(reloadText);
expect(reloadItemComponent).toBeInTheDocument();
await userEvent.click(reloadItemComponent);
await user.click(reloadItemComponent);

await waitFor(() => {
expect(reloadActionMock).toHaveBeenCalled();
Expand All @@ -83,7 +84,7 @@ describe('ApiErrorWrapper', () => {

const reloadItemComponent = screen.getByText(reloadText);
expect(reloadItemComponent).toBeInTheDocument();
await userEvent.click(reloadItemComponent);
await user.click(reloadItemComponent);

await waitFor(() => {
expect(window.location.reload).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import CodeInput from '../CodeInput';
const handleChangeMock = vi.fn();

describe('CodeInput Component', () => {
const user = userEvent.setup();

afterEach(() => {
vi.clearAllMocks();
});
Expand Down Expand Up @@ -82,28 +84,40 @@ describe('CodeInput Component', () => {
await waitFor(() => {
expect(handleChangeMock).toBeCalledTimes(2);
// check focus on next elem
expect(codeInputs[3]).toBe(document.activeElement);
expect(codeInputs[3]).toHaveFocus();
});
// change the value of the input and check that it is updated correctly
// set the cursor position to the end
act(() => (codeInputs[2] as HTMLInputElement).focus());
(codeInputs[2] as HTMLInputElement).setSelectionRange(1, 1);
// when we try to edit an input, we insert a second value and after, based on cursor position, we change the value
// we must use userEvent because the keyboard event must trigger also the change event (fireEvent doesn't do that)
await userEvent.keyboard('4');
await user.keyboard('4');
// next element will be focused
await waitFor(() => {
expect(codeInputs[3]).toHaveFocus();
});
await waitFor(() => {
expect(codeInputs[2]).toHaveValue('4');
});
// move the cursor at the start of the input and try to edit again
act(() => (codeInputs[2] as HTMLInputElement).focus());
(codeInputs[2] as HTMLInputElement).setSelectionRange(0, 0);
await userEvent.keyboard('3');
await user.keyboard('3');
// next element will be focused
await waitFor(() => {
expect(codeInputs[3]).toHaveFocus();
});
await waitFor(() => {
expect(codeInputs[2]).toHaveValue('3');
});
// delete the value
act(() => (codeInputs[2] as HTMLInputElement).focus());
await userEvent.keyboard('{Backspace}');
await user.keyboard('{Backspace}');
// previous element will be focused
await waitFor(() => {
expect(codeInputs[1]).toHaveFocus();
});
await waitFor(() => {
expect(codeInputs[2]).toHaveValue('');
});
Expand All @@ -120,52 +134,52 @@ describe('CodeInput Component', () => {
// press enter
fireEvent.keyDown(codeInputs[0], { key: 'Enter', code: 'Enter' });
await waitFor(() => {
expect(codeInputs[1]).toBe(document.activeElement);
expect(codeInputs[1]).toHaveFocus();
});
// press tab
fireEvent.keyDown(codeInputs[1], { key: 'Tab', code: 'Tab' });
await waitFor(() => {
expect(codeInputs[2]).toBe(document.activeElement);
expect(codeInputs[2]).toHaveFocus();
});
// press arrow right
fireEvent.keyDown(codeInputs[2], { key: 'ArrowRight', code: 'ArrowRight' });
await waitFor(() => {
expect(codeInputs[3]).toBe(document.activeElement);
expect(codeInputs[3]).toHaveFocus();
});
// press delete
fireEvent.keyDown(codeInputs[3], { key: 'Delete', code: 'Delete' });
await waitFor(() => {
expect(codeInputs[4]).toBe(document.activeElement);
expect(codeInputs[4]).toHaveFocus();
});
// press the same value of the input
// we reach the end, so we have to lost the focus
fireEvent.keyDown(codeInputs[4], { key: '1', code: 'Digit1' });
await waitFor(() => {
expect(document.body).toBe(document.activeElement);
expect(document.body).toHaveFocus();
});
// focus on last input and moove back
act(() => (codeInputs[4] as HTMLInputElement).focus());
// press backspace
fireEvent.keyDown(codeInputs[4], { key: 'Backspace', code: 'Backspace' });
await waitFor(() => {
expect(codeInputs[3]).toBe(document.activeElement);
expect(codeInputs[3]).toHaveFocus();
});
// press arrow left
fireEvent.keyDown(codeInputs[3], { key: 'ArrowLeft', code: 'ArrowLeft' });
await waitFor(() => {
expect(codeInputs[2]).toBe(document.activeElement);
expect(codeInputs[2]).toHaveFocus();
});
// press tab and shift key
fireEvent.keyDown(codeInputs[2], { key: 'Tab', code: 'Tab', shiftKey: true });
await waitFor(() => {
expect(codeInputs[1]).toBe(document.activeElement);
expect(codeInputs[1]).toHaveFocus();
});
// focus on first element and try to go back
act(() => (codeInputs[0] as HTMLInputElement).focus());
fireEvent.keyDown(codeInputs[0], { key: 'Backspace', code: 'Backspace' });
// nothing happens
await waitFor(() => {
expect(codeInputs[0]).toBe(document.activeElement);
expect(codeInputs[0]).toHaveFocus();
});
});
});
10 changes: 4 additions & 6 deletions packages/pn-commons/src/components/PnDialog/PnDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Children, cloneElement, isValidElement, useMemo } from 'react';
import { Children, cloneElement, isValidElement } from 'react';

import { Dialog, DialogProps, DialogTitle } from '@mui/material';

Expand All @@ -8,9 +8,8 @@ import PnDialogActions from './PnDialogActions';
import PnDialogContent from './PnDialogContent';

const PnDialog: React.FC<DialogProps> = (props) => {
const isMobile = useIsMobile();
const paddingSize = useMemo(() => (isMobile ? 3 : 4), [isMobile]);

const isMobile = useIsMobile('sm');
const paddingSize = isMobile ? 3 : 4;
const title: ReactComponent = Children.toArray(props.children).find(
(child) => isValidElement(child) && child.type === DialogTitle
);
Expand All @@ -26,11 +25,10 @@ const PnDialog: React.FC<DialogProps> = (props) => {
(child) => isValidElement(child) && child.type === PnDialogContent
);

const paddingTop = isMobile ? 3 : 4;
const enrichedContent = isValidElement(content)
? cloneElement(content, {
...content.props,
sx: { pt: title ? 0 : paddingTop, ...content.props.sx },
sx: { pt: title ? 0 : paddingSize, ...content.props.sx },
})
: content;

Expand Down
16 changes: 8 additions & 8 deletions packages/pn-commons/src/components/PnDialog/PnDialogActions.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { Children, cloneElement, isValidElement, useMemo } from 'react';
import { Children, cloneElement, isValidElement } from 'react';

import { Button, DialogActions, DialogActionsProps } from '@mui/material';

import { useIsMobile } from '../../hooks';
import { ReactComponent } from '../../models/PnDialog';

const PnDialogActions: React.FC<DialogActionsProps> = (props) => {
const isMobile = useIsMobile();
const paddingSize = useMemo(() => (isMobile ? 3 : 4), [isMobile]);
const gapSize = useMemo(() => (isMobile ? 0 : 1), [isMobile]);

const isMobile = useIsMobile('sm');
const buttons: Array<ReactComponent> | undefined = Children.toArray(props.children).filter(
(child) => isValidElement(child) && child.type === Button
);
Expand All @@ -19,7 +16,10 @@ const PnDialogActions: React.FC<DialogActionsProps> = (props) => {
? cloneElement(button, {
...button.props,
fullWidth: isMobile,
sx: { marginBottom: isMobile && index > 0 ? 2 : 0, ...button.props.sx },
sx: {
marginBottom: index > 0 && isMobile ? 2 : 0,
...button.props.sx,
},
})
: button
);
Expand All @@ -31,9 +31,9 @@ const PnDialogActions: React.FC<DialogActionsProps> = (props) => {
{...props}
sx={{
flexDirection: isMobile ? 'column-reverse' : 'row',
p: paddingSize,
p: isMobile ? 3 : 4,
pt: 0,
gap: gapSize,
gap: isMobile ? 0 : 1,
...props.sx,
}}
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { Children, cloneElement, isValidElement, useMemo } from 'react';
import { Children, cloneElement, isValidElement } from 'react';

import { DialogContent, DialogContentProps, DialogContentText } from '@mui/material';

import { useIsMobile } from '../../hooks';
import { ReactComponent } from '../../models/PnDialog';

const PnDialogContent: React.FC<DialogContentProps> = (props) => {
const isMobile = useIsMobile();
const paddingSize = useMemo(() => (isMobile ? 3 : 4), [isMobile]);

const isMobile = useIsMobile('sm');
const subtitle: ReactComponent = Children.toArray(props.children).find(
(child) => isValidElement(child) && child.type === DialogContentText
);
Expand All @@ -28,7 +26,7 @@ const PnDialogContent: React.FC<DialogContentProps> = (props) => {
<DialogContent
data-testid="dialog-content"
{...props}
sx={{ p: paddingSize, pt: 0, ...props.sx }}
sx={{ p: isMobile ? 3 : 4, pt: 0, ...props.sx }}
>
{subtitle && enrichedSubTitle}
{othersChildren}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('PnDialog Component', () => {
});

it('renders component - mobile', () => {
window.matchMedia = createMatchMedia(800);
window.matchMedia = createMatchMedia(500);
const { queryByTestId } = render(
<PnDialog open>
<DialogTitle data-testid="dialog-title">Title</DialogTitle>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('PnDialogActions Component', () => {
});

it('renders component - mobile', () => {
window.matchMedia = createMatchMedia(800);
window.matchMedia = createMatchMedia(500);
const { queryByTestId, queryAllByTestId } = render(
<PnDialogActions>
<Button data-testid="button">Test confirm button</Button>
Expand All @@ -41,8 +41,9 @@ describe('PnDialogActions Component', () => {
const buttons = queryAllByTestId('button');
expect(buttons).toHaveLength(2);
buttons.forEach((button, index) => {
expect(button).toHaveClass('MuiButton-fullWidth');
expect(button).toHaveStyle(index === 0 ? 'margin-bottom: 0' : 'margin-bottom: 16px');
expect(button).toHaveStyle(
index === 0 ? 'margin-bottom: 0' : 'margin-bottom: 16px; width: 100%'
);
});
});
});
6 changes: 3 additions & 3 deletions packages/pn-commons/src/hooks/useIsMobile.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { useTheme } from '@mui/material';
import { Breakpoint, useTheme } from '@mui/material';
import useMediaQuery from '@mui/material/useMediaQuery';

/**
* Checks if we are on a mobile device
*/
export const useIsMobile = () => {
export const useIsMobile = (breakpoint: Breakpoint | number = 'lg') => {
const theme = useTheme();
return useMediaQuery(theme.breakpoints.down('lg'));
return useMediaQuery(theme.breakpoints.down(breakpoint));
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useState } from 'react';
import { Box, Button, DialogTitle, Grid, Typography } from '@mui/material';
import { useTranslation } from 'react-i18next';

import { Box, Button, DialogTitle, Grid, Typography } from '@mui/material';
import {
CollapsedList,
NotificationDetailRecipient,
Expand Down

0 comments on commit 2eda2f6

Please sign in to comment.