Skip to content

Commit

Permalink
Warn user when leaving bpmn editor with unsaved changes (#11715)
Browse files Browse the repository at this point in the history
* Upgrade react-router-dom from 6.18.0 to 6.20.0

* Refactor routes

* Create new hook to block navigation

* Implement new hook

* Move App code into RouterProvider

* Add tests for useConfirmationNavigation hook

* Fix unit tests

* Fix Canvas unit tests

* Rename hook to useConfirmationDialogOnPageLeave

* Rename AppShell and Layout

* Fix missing type

* Add missing changes

* Fixes after cr
  • Loading branch information
mlqn authored Dec 13, 2023
1 parent 71d7acc commit 5586654
Show file tree
Hide file tree
Showing 23 changed files with 329 additions and 120 deletions.
16 changes: 6 additions & 10 deletions frontend/app-development/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ import { createRoot } from 'react-dom/client';
import { Provider } from 'react-redux';
import { run } from './sagas';
import { setupStore } from './store';
import { BrowserRouter } from 'react-router-dom';
import { App } from './App';
import { APP_DEVELOPMENT_BASENAME } from 'app-shared/constants';
import { ServicesContextProvider } from 'app-shared/contexts/ServicesContext';
import * as queries from 'app-shared/api/queries';
import * as mutations from 'app-shared/api/mutations';
Expand All @@ -15,6 +12,7 @@ import { LoggerConfig, LoggerContextProvider } from 'app-shared/contexts/LoggerC
import 'app-shared/design-tokens';
import { altinnStudioEnvironment } from 'app-shared/utils/altinnStudioEnv';
import { QueryClientConfig } from '@tanstack/react-query';
import { PageRoutes } from './router/PageRoutes';

const store = setupStore();

Expand Down Expand Up @@ -44,13 +42,11 @@ const queryClientConfig: QueryClientConfig = {
root.render(
<LoggerContextProvider config={loggerConfig}>
<Provider store={store}>
<BrowserRouter basename={APP_DEVELOPMENT_BASENAME}>
<ServicesContextProvider clientConfig={queryClientConfig} {...queries} {...mutations}>
<PreviewConnectionContextProvider>
<App />
</PreviewConnectionContextProvider>
</ServicesContextProvider>
</BrowserRouter>
<ServicesContextProvider clientConfig={queryClientConfig} {...queries} {...mutations}>
<PreviewConnectionContextProvider>
<PageRoutes />
</PreviewConnectionContextProvider>
</ServicesContextProvider>
</Provider>
</LoggerContextProvider>,
);
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
);
--left-menu-width: 68px;

background-color: lightgray;
background: var(--fds-semantic-background-default);
min-height: 100vh;
width: 100%;
display: flex;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import React from 'react';
import { App } from './App';
import type { IUserState } from './sharedResources/user/userSlice';
import type { IUserState } from '../sharedResources/user/userSlice';
import { screen } from '@testing-library/react';
import { APP_DEVELOPMENT_BASENAME } from 'app-shared/constants';
import { renderWithProviders } from './test/testUtils';
import * as testids from '../testing/testids';
import { textMock } from '../testing/mocks/i18nMock';
import { renderWithProviders } from '../test/testUtils';
import * as testids from '../../testing/testids';
import { textMock } from '../../testing/mocks/i18nMock';
import { queriesMock } from 'app-shared/mocks/queriesMock';

jest.mock('../language/src/nb.json', jest.fn());
jest.mock('../language/src/en.json', jest.fn());
jest.mock('../../language/src/nb.json', jest.fn());
jest.mock('../../language/src/en.json', jest.fn());

// Mocking console.error due to Tanstack Query removing custom logger between V4 and v5 see issue: #11692
const realConsole = console;
Expand All @@ -26,6 +26,7 @@ const render = async (remainingMinutes: number = 40) => {
},
});
};

describe('App', () => {
beforeEach(() => {
global.console = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import React, { useCallback, useEffect, useRef } from 'react';
import postMessages from 'app-shared/utils/postMessages';
import { AltinnPopoverSimple } from 'app-shared/components/molecules/AltinnPopoverSimple';
import { HandleServiceInformationActions } from './features/overview/handleServiceInformationSlice';
import { HandleServiceInformationActions } from '../features/overview/handleServiceInformationSlice';
import {
fetchRemainingSession,
keepAliveSession,
signOutUser,
} from './sharedResources/user/userSlice';
} from '../sharedResources/user/userSlice';
import './App.css';
import { matchPath, useLocation } from 'react-router-dom';
import { Outlet, matchPath, useLocation } from 'react-router-dom';
import classes from './App.module.css';
import { useAppDispatch, useAppSelector } from './hooks';
import { useAppDispatch, useAppSelector } from '../hooks';
import { getRepositoryType } from 'app-shared/utils/repository';
import { RepositoryType } from 'app-shared/types/global';
import {
Expand All @@ -21,12 +21,11 @@ import {
} from 'app-shared/api/paths';
import i18next from 'i18next';
import { initReactI18next, useTranslation } from 'react-i18next';
import nb from '../language/src/nb.json';
import en from '../language/src/en.json';
import nb from '../../language/src/nb.json';
import en from '../../language/src/en.json';
import { DEFAULT_LANGUAGE } from 'app-shared/constants';
import { useRepoStatusQuery } from 'app-shared/hooks/queries';
import * as testids from '../testing/testids';
import { PageRoutes } from './router/PageRoutes';
import * as testids from '../../testing/testids';

const TEN_MINUTES_IN_MILLISECONDS = 600000;

Expand Down Expand Up @@ -158,7 +157,7 @@ export function App() {
<p style={{ marginTop: '1.6rem' }}>{t('session.inactive')}</p>
</AltinnPopoverSimple>
<div data-testid={testids.appContentWrapper}>
<PageRoutes />
<Outlet />
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { AppShell } from './AppShell';
import { PageLayout } from './PageLayout';
import { screen, waitForElementToBeRemoved } from '@testing-library/react';
import { APP_DEVELOPMENT_BASENAME } from 'app-shared/constants';
import { renderWithProviders } from '../test/testUtils';
Expand Down Expand Up @@ -43,7 +43,7 @@ jest.mock('react-router-dom', () => ({
// Mocking console.error due to Tanstack Query removing custom logger between V4 and v5 see issue: #11692
const realConsole = console;

describe('App', () => {
describe('PageLayout', () => {
beforeEach(() => {
global.console = {
...console,
Expand Down Expand Up @@ -112,7 +112,7 @@ const render = async (queries: Partial<ServicesContextProps> = {}) => {
...queries,
};

renderWithProviders(<AppShell />, {
renderWithProviders(<PageLayout />, {
startUrl: `${APP_DEVELOPMENT_BASENAME}/my-org/my-app/${RoutePaths.Overview}`,
queries: allQueries,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MergeConflictWarning } from '../features/simpleMerge/MergeConflictWarni
/**
* Displays the layout for the app development pages
*/
export const AppShell = (): React.ReactNode => {
export const PageLayout = (): React.ReactNode => {
const { pathname } = useLocation();
const match = matchPath({ path: '/:org/:app', caseSensitive: true, end: false }, pathname);
const { org, app } = match.params;
Expand Down
2 changes: 1 addition & 1 deletion frontend/app-development/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"react-dom": "18.2.0",
"react-i18next": "13.3.1",
"react-redux": "8.1.3",
"react-router-dom": "6.18.0",
"react-router-dom": "6.20.0",
"redux": "4.2.1",
"redux-saga": "1.2.3",
"reselect": "4.1.8"
Expand Down
6 changes: 0 additions & 6 deletions frontend/app-development/router/PageRoutes.module.css

This file was deleted.

50 changes: 30 additions & 20 deletions frontend/app-development/router/PageRoutes.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,40 @@
import React from 'react';
import classes from './PageRoutes.module.css';
import { Navigate, Route, Routes } from 'react-router-dom';
import { AppShell } from 'app-development/layout/AppShell';
import {
RouterProvider,
createBrowserRouter,
createRoutesFromElements,
Navigate,
Route,
} from 'react-router-dom';
import { App } from 'app-development/layout/App';
import { PageLayout } from 'app-development/layout/PageLayout';
import { RoutePaths } from 'app-development/enums/RoutePaths';
import { routerRoutes } from 'app-development/router/routes';
import { StudioNotFoundPage } from '@studio/components';
import { APP_DEVELOPMENT_BASENAME } from 'app-shared/constants';

const BASE_PATH = '/:org/:app';

const router = createBrowserRouter(
createRoutesFromElements(
<Route path='/' element={<App />}>
<Route path={BASE_PATH} element={<PageLayout />}>
{/* Redirects from /:org/:app to child route /overview */}
<Route path={RoutePaths.Root} element={<Navigate to={RoutePaths.Overview} />} />
{routerRoutes.map((route) => (
<Route key={route.path} path={route.path} element={<route.subapp {...route.props} />} />
))}
<Route path='*' element={<StudioNotFoundPage />} />
</Route>
<Route path='*' element={<StudioNotFoundPage />} />
</Route>,
),
{
basename: APP_DEVELOPMENT_BASENAME,
},
);

/**
* Displays the routes for app development pages
*/
export const PageRoutes = () => {
return (
<div className={classes.root}>
<Routes>
<Route path={BASE_PATH} element={<AppShell />}>
{/* Redirects from /:org/:app to child route /overview */}
<Route path={RoutePaths.Root} element={<Navigate to={RoutePaths.Overview} />} />
{routerRoutes.map((route) => (
<Route key={route.path} path={route.path} element={<route.subapp {...route.props} />} />
))}
<Route path='*' element={<StudioNotFoundPage />} />
</Route>
<Route path='*' element={<StudioNotFoundPage />} />
</Routes>
</div>
);
};
export const PageRoutes = () => <RouterProvider router={router} />;
2 changes: 1 addition & 1 deletion frontend/app-preview/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"react": "18.2.0",
"react-dom": "18.2.0",
"react-redux": "8.1.3",
"react-router-dom": "6.18.0"
"react-router-dom": "6.20.0"
},
"devDependencies": {
"cross-env": "7.0.3",
Expand Down
2 changes: 1 addition & 1 deletion frontend/dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"@mui/x-data-grid": "5.17.26",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-router-dom": "6.18.0",
"react-router-dom": "6.20.0",
"react-use": "17.4.0"
},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@
"process_editor.unknown_heading_error_message": "Obs, noe gikk galt!",
"process_editor.unknown_paragraph_error_message": "En feil oppstod ved innlasting av BPMN-prosessen. Undersøk at filen er på et gyldig BPMN-format.",
"process_editor.unsaved_changes": "Du har {{ count }} ulagrede endringer",
"process_editor.unsaved_changes_confirmation_message": "Du har ulagrede endringer. Vil du forlate siden uten å lagre?",
"process_editor.view_mode": "Visningsmodus",
"receipt.attachments": "Vedlegg",
"receipt.body": "Det er gjennomført en maskinell kontroll under utfylling, men vi tar forbehold om at det kan bli oppdaget feil under saksbehandlingen og at annen dokumentasjon kan være nødvendig. Vennligst oppgi referansenummer ved eventuelle henvendelser til etaten.",
Expand Down
37 changes: 25 additions & 12 deletions frontend/packages/process-editor/src/ProcessEditor.test.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react';
import { render, screen, act } from '@testing-library/react';
import { render as rtlRender, screen, act } from '@testing-library/react';
import { ProcessEditor, ProcessEditorProps } from './ProcessEditor';
import { textMock } from '../../../testing/mocks/i18nMock';
import userEvent from '@testing-library/user-event';
import { RouterProvider, createMemoryRouter } from 'react-router-dom';

const mockBPMNXML: string = `<?xml version="1.0" encoding="UTF-8"?></xml>`;

Expand All @@ -11,22 +12,34 @@ const mockAppLibVersion7: string = '7.0.3';

const mockOnSave = jest.fn();

const defaultProps: ProcessEditorProps = {
bpmnXml: mockBPMNXML,
onSave: mockOnSave,
appLibVersion: mockAppLibVersion8,
};

const render = (props: Partial<ProcessEditorProps> = {}) => {
const allProps = { ...defaultProps, ...props };
const router = createMemoryRouter([
{
path: '/',
element: <ProcessEditor {...allProps} />,
},
]);

return rtlRender(<RouterProvider router={router}></RouterProvider>);
};

describe('ProcessEditor', () => {
afterEach(jest.clearAllMocks);

const defaultProps: ProcessEditorProps = {
bpmnXml: mockBPMNXML,
onSave: mockOnSave,
appLibVersion: mockAppLibVersion8,
};

it('should render loading while bpmnXml is undefined', () => {
render(<ProcessEditor {...defaultProps} bpmnXml={undefined} />);
render({ bpmnXml: undefined });
expect(screen.getByTitle(textMock('process_editor.loading'))).toBeInTheDocument();
});

it('should render "NoBpmnFoundAlert" when bpmnXml is null', () => {
render(<ProcessEditor {...defaultProps} bpmnXml={null} />);
render({ bpmnXml: null });
expect(
screen.getByRole('heading', {
name: textMock('process_editor.fetch_bpmn_error_title'),
Expand All @@ -38,7 +51,7 @@ describe('ProcessEditor', () => {
it('should render "canvas" when bpmnXml is provided and default render is view-mode', async () => {
// eslint-disable-next-line testing-library/no-unnecessary-act
await act(() => {
render(<ProcessEditor {...defaultProps} />);
render();
});

expect(
Expand All @@ -48,7 +61,7 @@ describe('ProcessEditor', () => {

it('does not display the alert when the version is 8 or newer', async () => {
const user = userEvent.setup();
render(<ProcessEditor {...defaultProps} />);
render();

// Fix to remove act error
await act(() => user.tab());
Expand All @@ -62,7 +75,7 @@ describe('ProcessEditor', () => {

it('displays the alert when the version is 7 or older', async () => {
const user = userEvent.setup();
render(<ProcessEditor {...defaultProps} appLibVersion={mockAppLibVersion7} />);
render({ appLibVersion: mockAppLibVersion7 });

// Fix to remove act error
await act(() => user.tab());
Expand Down
Loading

0 comments on commit 5586654

Please sign in to comment.