Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/PN-9620: fix spacing in modal PF/PA/PG for tablet #1166

Merged
merged 14 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading