From 80f8cfa4e71a73d5c3876432f278a11bc7f0ae51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 22 Jan 2024 11:53:56 +0100 Subject: [PATCH 1/2] CatchError: Provide theme when rendering caught error --- web/packages/teleterm/src/ui/App.tsx | 29 +--------- web/packages/teleterm/src/ui/boot.tsx | 3 +- .../teleterm/src/ui/components/App.test.tsx | 29 ++++++++++ .../teleterm/src/ui/components/App.tsx | 46 +++++++++++++++ .../ui/components/CatchError/CatchError.jsx | 6 +- .../components/CatchError/CatchError.test.tsx | 58 +++++++++++++++++++ .../src/ui/components/CatchError/index.js | 3 +- 7 files changed, 142 insertions(+), 32 deletions(-) create mode 100644 web/packages/teleterm/src/ui/components/App.test.tsx create mode 100644 web/packages/teleterm/src/ui/components/App.tsx create mode 100644 web/packages/teleterm/src/ui/components/CatchError/CatchError.test.tsx diff --git a/web/packages/teleterm/src/ui/App.tsx b/web/packages/teleterm/src/ui/App.tsx index 25889987f293a..2381dee4d35a2 100644 --- a/web/packages/teleterm/src/ui/App.tsx +++ b/web/packages/teleterm/src/ui/App.tsx @@ -17,17 +17,14 @@ import React from 'react'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; -import styled from 'styled-components'; - -import { Failed } from 'design/CardError'; import { AppInitializer } from 'teleterm/ui/AppInitializer'; -import CatchError from './components/CatchError'; +import { CatchError } from './components/CatchError'; +import { StyledApp } from './components/App'; import AppContextProvider from './appContextProvider'; import AppContext from './appContext'; -import { StaticThemeProvider, ThemeProvider } from './ThemeProvider'; -import { darkTheme } from './ThemeProvider/theme'; +import { ThemeProvider } from './ThemeProvider'; export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { return ( @@ -44,23 +41,3 @@ export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { ); }; - -export const FailedApp = (props: { message: string }) => { - return ( - - - - - - ); -}; - -const StyledApp = styled.div` - left: 0; - right: 0; - top: 0; - bottom: 0; - position: absolute; - display: flex; - flex-direction: column; -`; diff --git a/web/packages/teleterm/src/ui/boot.tsx b/web/packages/teleterm/src/ui/boot.tsx index 4ffc92007c528..e79d4ad2230ac 100644 --- a/web/packages/teleterm/src/ui/boot.tsx +++ b/web/packages/teleterm/src/ui/boot.tsx @@ -18,7 +18,8 @@ import ReactDOM from 'react-dom'; import React from 'react'; import { ElectronGlobals } from 'teleterm/types'; -import { App, FailedApp } from 'teleterm/ui/App'; +import { App } from 'teleterm/ui/App'; +import { FailedApp } from 'teleterm/ui/components/App'; import AppContext from 'teleterm/ui/appContext'; import Logger from 'teleterm/logger'; diff --git a/web/packages/teleterm/src/ui/components/App.test.tsx b/web/packages/teleterm/src/ui/components/App.test.tsx new file mode 100644 index 0000000000000..6ef9d04853b3e --- /dev/null +++ b/web/packages/teleterm/src/ui/components/App.test.tsx @@ -0,0 +1,29 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { screen, render } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import { FailedApp } from './App'; + +describe('', () => { + it('renders (without ThemeProvider being available)', () => { + render(); + expect(screen.getByText('Lorem ipsum')).toBeInTheDocument(); + }); +}); diff --git a/web/packages/teleterm/src/ui/components/App.tsx b/web/packages/teleterm/src/ui/components/App.tsx new file mode 100644 index 0000000000000..15ca0aae7529f --- /dev/null +++ b/web/packages/teleterm/src/ui/components/App.tsx @@ -0,0 +1,46 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +import styled from 'styled-components'; +import { Failed } from 'design/CardError'; + +import { StaticThemeProvider } from 'teleterm/ui/ThemeProvider'; +import { darkTheme } from 'teleterm/ui/ThemeProvider/theme'; + +export const StyledApp = styled.div` + left: 0; + right: 0; + top: 0; + bottom: 0; + position: absolute; + display: flex; + flex-direction: column; +`; + +export const FailedApp = (props: { message: string }) => { + return ( + + {/* + FailedApp is used above ThemeProvider in the component hierarchy. Since it cannot depend on + ThemeProvider to provide a theme, it needs to use StaticThemeProvider to provide one. + */} + + + + + ); +}; diff --git a/web/packages/teleterm/src/ui/components/CatchError/CatchError.jsx b/web/packages/teleterm/src/ui/components/CatchError/CatchError.jsx index 5185607a70a25..4eb520b78a059 100644 --- a/web/packages/teleterm/src/ui/components/CatchError/CatchError.jsx +++ b/web/packages/teleterm/src/ui/components/CatchError/CatchError.jsx @@ -15,11 +15,11 @@ limitations under the License. */ import React from 'react'; -import { Failed } from 'design/CardError'; +import { FailedApp } from 'teleterm/ui/components/App'; import Logger from 'teleterm/logger'; -export default class CatchError extends React.Component { +export class CatchError extends React.Component { logger = new Logger('components/CatchError'); static getDerivedStateFromError(error) { @@ -37,7 +37,7 @@ export default class CatchError extends React.Component { render() { if (this.state.error) { return ( - + ); } diff --git a/web/packages/teleterm/src/ui/components/CatchError/CatchError.test.tsx b/web/packages/teleterm/src/ui/components/CatchError/CatchError.test.tsx new file mode 100644 index 0000000000000..436e840f2eaf5 --- /dev/null +++ b/web/packages/teleterm/src/ui/components/CatchError/CatchError.test.tsx @@ -0,0 +1,58 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { screen, render } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import Logger, { NullService } from 'teleterm/logger'; + +import { CatchError } from './CatchError'; + +beforeAll(() => { + Logger.init(new NullService()); +}); + +beforeEach(() => { + jest.restoreAllMocks(); + // Mock console.error, otherwise the test would output noise to the terminal. + jest.spyOn(console, 'error').mockImplementation(() => {}); +}); + +const ThrowError = (props: { error: any }) => { + throw props.error; +}; + +it('renders caught error (without ThemeProvider being available)', () => { + render( + + + + ); + expect(screen.getByText('Lorem ipsum')).toBeInTheDocument(); + expect(console.error).toHaveBeenCalled(); +}); + +it('works correctly when a non-Error value is thrown', () => { + render( + + + + ); + expect(screen.getByText('Lorem ipsum')).toBeInTheDocument(); + expect(console.error).toHaveBeenCalled(); +}); diff --git a/web/packages/teleterm/src/ui/components/CatchError/index.js b/web/packages/teleterm/src/ui/components/CatchError/index.js index 992778fc76f95..ccb6b6d6586e3 100644 --- a/web/packages/teleterm/src/ui/components/CatchError/index.js +++ b/web/packages/teleterm/src/ui/components/CatchError/index.js @@ -14,5 +14,4 @@ See the License for the specific language governing permissions and limitations under the License. */ -import CatchError from './CatchError'; -export default CatchError; +export { CatchError } from './CatchError'; From de30835ad40431195c79c2fee4ba19fed1967577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 22 Jan 2024 12:07:12 +0100 Subject: [PATCH 2/2] Move CatchError to top of App hierarchy to avoid double render of StyledApp --- web/packages/teleterm/src/ui/App.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/packages/teleterm/src/ui/App.tsx b/web/packages/teleterm/src/ui/App.tsx index 2381dee4d35a2..57e5a808374f3 100644 --- a/web/packages/teleterm/src/ui/App.tsx +++ b/web/packages/teleterm/src/ui/App.tsx @@ -28,8 +28,8 @@ import { ThemeProvider } from './ThemeProvider'; export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { return ( - - + + @@ -37,7 +37,7 @@ export const App: React.FC<{ ctx: AppContext }> = ({ ctx }) => { - - + + ); };