From 0b632445a933644c7eb10015eb04b7c21d562900 Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Sun, 24 Apr 2022 17:01:25 +0200 Subject: refactor: reduce service switcher tearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We render the location bar and notification banners separately for each service and keep track of the BrowserView size separately for each service to reduce the tearing that appears when people switch services. The tearing cannot be eliminated completely, because it comes from the separation between the main and renderer processes. But we can at least try and reduce the IPC round-tripping and layout calculations required to accurately position the services. This approach has an overhead compared to the single BrowserViewPlaceholder approach, because the renderer process has to layout the location bar and notification for all services, not only the selected one. The number of IPC messages during windows resize is also increased. To compensate, we increase the throttle interval for resize IPC messages and let electron itself resize the BrowserView between IPC updates. (We must still keep pumping IPC messages during window resize, because, e.g., changes in notification banner size due to re-layouting will still affect the required BrowserView size). If further reduction of IPC traffic is needed, we could implement batching for resize IPC messages and more intelligent throttling via a token bucker mechanism. Signed-off-by: Kristóf Marussy --- packages/renderer/src/components/App.tsx | 22 ++----- .../src/components/BrowserViewPlaceholder.tsx | 14 ++-- packages/renderer/src/components/ServicePanel.tsx | 76 ++++++++++++++++++++++ .../components/banner/InsecureConnectionBanner.tsx | 7 +- .../src/components/banner/NewWindowBanner.tsx | 7 +- .../src/components/errorPage/ErrorPage.tsx | 8 +-- .../src/components/locationBar/ExtraButtons.tsx | 10 +-- .../src/components/locationBar/LocationBar.tsx | 24 ++++--- .../components/locationBar/LocationTextField.tsx | 21 +++--- .../components/locationBar/NavigationButtons.tsx | 27 +++----- .../src/components/sidebar/ServiceSwitcher.tsx | 2 + .../components/sidebar/ToggleLocationBarButton.tsx | 8 ++- 12 files changed, 137 insertions(+), 89 deletions(-) create mode 100644 packages/renderer/src/components/ServicePanel.tsx (limited to 'packages/renderer/src/components') diff --git a/packages/renderer/src/components/App.tsx b/packages/renderer/src/components/App.tsx index d381abf..2f728e7 100644 --- a/packages/renderer/src/components/App.tsx +++ b/packages/renderer/src/components/App.tsx @@ -23,12 +23,8 @@ import { observer } from 'mobx-react-lite'; import React, { useCallback, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; -import BrowserViewPlaceholder from './BrowserViewPlaceholder'; +import ServicePanel from './ServicePanel'; import { useStore } from './StoreProvider'; -import InsecureConnectionBanner from './banner/InsecureConnectionBanner'; -import NewWindowBanner from './banner/NewWindowBanner'; -import ErrorPage from './errorPage/ErrorPage'; -import LocationBar from './locationBar/LocationBar'; import Sidebar from './sidebar/Sidebar'; function App({ devMode }: { devMode: boolean }): JSX.Element { @@ -37,6 +33,7 @@ function App({ devMode }: { devMode: boolean }): JSX.Element { }); const { settings: { selectedService }, + shared: { services }, } = useStore(); const { settings: { name: serviceName }, @@ -95,20 +92,13 @@ function App({ devMode }: { devMode: boolean }): JSX.Element { - - - - - - + {services.map((service) => ( + + ))} ); diff --git a/packages/renderer/src/components/BrowserViewPlaceholder.tsx b/packages/renderer/src/components/BrowserViewPlaceholder.tsx index 9bd1176..2bfc9b0 100644 --- a/packages/renderer/src/components/BrowserViewPlaceholder.tsx +++ b/packages/renderer/src/components/BrowserViewPlaceholder.tsx @@ -22,29 +22,29 @@ import Box from '@mui/material/Box'; import throttle from 'lodash-es/throttle'; import React, { ReactNode, useCallback, useRef } from 'react'; -import { useStore } from './StoreProvider'; +import Service from '../stores/Service'; function BrowserViewPlaceholder({ + service, children, }: { + service: Service; children?: ReactNode; }): JSX.Element { - const store = useStore(); - // eslint-disable-next-line react-hooks/exhaustive-deps -- react-hooks doesn't support `throttle`. const onResize = useCallback( throttle(([entry]: ResizeObserverEntry[]) => { if (entry) { const { x, y, width, height } = entry.target.getBoundingClientRect(); - store.setBrowserViewBounds({ + service.setBrowserViewBounds({ x: Math.round(x), y: Math.round(y), width: Math.round(width), height: Math.round(height), }); } - }, 40), - [store], + }, 100), + [service], ); const resizeObserverRef = useRef(); @@ -61,7 +61,7 @@ function BrowserViewPlaceholder({ resizeObserverRef.current = new ResizeObserver(onResize); resizeObserverRef.current.observe(element); }, - [onResize, resizeObserverRef], + [onResize], ); return ( diff --git a/packages/renderer/src/components/ServicePanel.tsx b/packages/renderer/src/components/ServicePanel.tsx new file mode 100644 index 0000000..de58d24 --- /dev/null +++ b/packages/renderer/src/components/ServicePanel.tsx @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2021-2022 Kristóf Marussy + * + * This file is part of Sophie. + * + * Sophie 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, version 3. + * + * 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 . + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import Box from '@mui/material/Box'; +import { observer } from 'mobx-react-lite'; +import React from 'react'; + +import Service from '../stores/Service'; + +import BrowserViewPlaceholder from './BrowserViewPlaceholder'; +import { useStore } from './StoreProvider'; +import InsecureConnectionBanner from './banner/InsecureConnectionBanner'; +import NewWindowBanner from './banner/NewWindowBanner'; +import ErrorPage from './errorPage/ErrorPage'; +import LocationBar from './locationBar/LocationBar'; + +export function getServicePanelID(service: Service): string { + return `Sophie-${service.id}-ServicePanel`; +} + +function ServicePanel({ service }: { service: Service }): JSX.Element { + const { + settings: { selectedService }, + } = useStore(); + + const { + settings: { name }, + } = service; + const visible = service === selectedService; + + return ( + + + + + + + + + ); +} + +export default observer(ServicePanel); diff --git a/packages/renderer/src/components/banner/InsecureConnectionBanner.tsx b/packages/renderer/src/components/banner/InsecureConnectionBanner.tsx index 7a03fce..0b70db6 100644 --- a/packages/renderer/src/components/banner/InsecureConnectionBanner.tsx +++ b/packages/renderer/src/components/banner/InsecureConnectionBanner.tsx @@ -34,17 +34,12 @@ import NotificationBanner from './NotificationBanner'; function InsecureConnectionBanner({ service, }: { - service: Service | undefined; + service: Service; }): JSX.Element | null { const { t } = useTranslation(undefined, { keyPrefix: 'banner.insecureConnection', }); - if (service === undefined) { - // eslint-disable-next-line unicorn/no-null -- React requires `null` to skip rendering. - return null; - } - const { canReconnectSecurely, hasError, diff --git a/packages/renderer/src/components/banner/NewWindowBanner.tsx b/packages/renderer/src/components/banner/NewWindowBanner.tsx index 478de8e..07fafda 100644 --- a/packages/renderer/src/components/banner/NewWindowBanner.tsx +++ b/packages/renderer/src/components/banner/NewWindowBanner.tsx @@ -32,17 +32,12 @@ import NotificationBanner from './NotificationBanner'; function NewWindowBanner({ service, }: { - service: Service | undefined; + service: Service; }): JSX.Element | null { const { t } = useTranslation(undefined, { keyPrefix: 'banner.newWindow', }); - if (service === undefined) { - // eslint-disable-next-line unicorn/no-null -- React requires `null` to skip rendering. - return null; - } - const { popups, settings: { name }, diff --git a/packages/renderer/src/components/errorPage/ErrorPage.tsx b/packages/renderer/src/components/errorPage/ErrorPage.tsx index 99ca020..10f54cd 100644 --- a/packages/renderer/src/components/errorPage/ErrorPage.tsx +++ b/packages/renderer/src/components/errorPage/ErrorPage.tsx @@ -123,14 +123,10 @@ function formatError(service: Service, t: TFunction): ErrorDetails { } } -function ErrorPage({ - service, -}: { - service: Service | undefined; -}): JSX.Element | null { +function ErrorPage({ service }: { service: Service }): JSX.Element | null { const { t } = useTranslation(undefined); - if (service === undefined || !service.hasError) { + if (!service.hasError) { // eslint-disable-next-line unicorn/no-null -- React requires `null` to skip rendering. return null; } diff --git a/packages/renderer/src/components/locationBar/ExtraButtons.tsx b/packages/renderer/src/components/locationBar/ExtraButtons.tsx index ef90199..4d4c3c4 100644 --- a/packages/renderer/src/components/locationBar/ExtraButtons.tsx +++ b/packages/renderer/src/components/locationBar/ExtraButtons.tsx @@ -27,11 +27,7 @@ import { useTranslation } from 'react-i18next'; import type Service from '../../stores/Service'; -function ExtraButtons({ - service, -}: { - service: Service | undefined; -}): JSX.Element { +function ExtraButtons({ service }: { service: Service }): JSX.Element { const { t } = useTranslation(undefined, { keyPrefix: 'toolbar', }); @@ -40,8 +36,8 @@ function ExtraButtons({ service?.openCurrentURLInExternalBrowser()} + disabled={service.currentUrl === undefined} + onClick={() => service.openCurrentURLInExternalBrowser()} > diff --git a/packages/renderer/src/components/locationBar/LocationBar.tsx b/packages/renderer/src/components/locationBar/LocationBar.tsx index 54ead8e..c290722 100644 --- a/packages/renderer/src/components/locationBar/LocationBar.tsx +++ b/packages/renderer/src/components/locationBar/LocationBar.tsx @@ -22,13 +22,16 @@ import { styled } from '@mui/material/styles'; import { observer } from 'mobx-react-lite'; import React from 'react'; +import type Service from '../../stores/Service'; import { useStore } from '../StoreProvider'; import ExtraButtons from './ExtraButtons'; import LocationTextField from './LocationTextField'; import NavigationButtons from './NavigationButtons'; -export const LOCATION_BAR_ID = 'Sophie-LocationBar'; +export function getLocaltionBarID(service: Service): string { + return `Sophie-${service.id}-LocationBar`; +} const LocationBarRoot = styled('header', { name: 'LocationBar', @@ -41,17 +44,22 @@ const LocationBarRoot = styled('header', { borderBottom: `1px solid ${theme.palette.divider}`, })); -function LocationBar(): JSX.Element { +function LocationBar({ service }: { service: Service }): JSX.Element { const { - shared: { locationBarVisible }, - settings: { selectedService }, + settings: { showLocationBar }, } = useStore(); + const { alwaysShowLocationBar } = service; + const locationBarVisible = showLocationBar || alwaysShowLocationBar; + return ( -