From 81eb09e7b00472050709fbe074a011a952d28756 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Fri, 21 Aug 2020 10:05:19 -0700 Subject: [PATCH] Unify error notifications (#1483) Summary: Note: this is to be stacked upon https://github.com/facebook/flipper/pull/1479 Note: this PR will probably not succeed against FB internal flipper, as I'm pretty sure there are more call sites that need to be updated. So consider this WIP Currently connection errors are managed in the connection reducers, and are displayed through their own means, the error bar. Showing console.errors is also hooked up to this mechanism in FB internal flipper, but not at all in the OSS version, which means that some connection errors are never shown to the user. Besides that there is a notification system that is used by for example the crash reporter and plugin updater. Having effectively (at least) two notifications mechanisms is confusing and error prone. This PR unifies both approaches, and rather than having the connection reducer manage it's own errors, it leverages the more generic notifications reducer. Since, in the previous PR, console errors and warnings have become user facing (even in OSS and production builds, which wasn't the case before), there is no need anymore for a separate error bar. I left the notifications mechanism itself as-is, but as discussed in the Sandy project the notification screen will probably be overhauled, and the system wide notifications will become in-app notifications. ## Changelog Pull Request resolved: https://github.com/facebook/flipper/pull/1483 Test Plan: Only updated the unit tests at this point. Manual tests still need to be done. Reviewed By: passy Differential Revision: D23220896 Pulled By: mweststrate fbshipit-source-id: 8ea37cf69ce9605dc232ca90afe9e2f70da26652 --- desktop/app/src/App.tsx | 8 +- .../createMockFlipperWithPlugin.node.tsx.snap | 1 - desktop/app/src/chrome/ErrorBar.tsx | 200 ------------------ desktop/app/src/dispatcher/androidDevice.tsx | 24 +-- desktop/app/src/dispatcher/iOSDevice.tsx | 17 +- desktop/app/src/dispatcher/metroDevice.tsx | 16 +- desktop/app/src/dispatcher/server.tsx | 33 +-- .../reducers/__tests__/connections.node.tsx | 86 -------- .../reducers/__tests__/notifications.node.tsx | 65 +++++- desktop/app/src/reducers/connections.tsx | 87 -------- desktop/app/src/reducers/notifications.tsx | 31 ++- 11 files changed, 133 insertions(+), 435 deletions(-) delete mode 100644 desktop/app/src/chrome/ErrorBar.tsx diff --git a/desktop/app/src/App.tsx b/desktop/app/src/App.tsx index c68895ad5..ec8f0d4ae 100644 --- a/desktop/app/src/App.tsx +++ b/desktop/app/src/App.tsx @@ -12,7 +12,6 @@ import {FlexRow, styled, Layout} from 'flipper'; import {connect} from 'react-redux'; import TitleBar from './chrome/TitleBar'; import MainSidebar2 from './chrome/mainsidebar/MainSidebar2'; -import ErrorBar from './chrome/ErrorBar'; import DoctorBar from './chrome/DoctorBar'; import ShareSheetExportUrl from './chrome/ShareSheetExportUrl'; import SignInSheet from './chrome/SignInSheet'; @@ -40,7 +39,7 @@ import { } from './reducers/application'; import {Logger} from './fb-interfaces/Logger'; import {State as Store} from './reducers/index'; -import {StaticView, FlipperError} from './reducers/connections'; +import {StaticView} from './reducers/connections'; import PluginManager from './chrome/plugin-manager/PluginManager'; import StatusBar from './chrome/StatusBar'; import SettingsSheet from './chrome/SettingsSheet'; @@ -59,7 +58,6 @@ type OwnProps = { type StateFromProps = { leftSidebarVisible: boolean; - errors: FlipperError[]; activeSheet: ActiveSheet; share: ShareType | null; staticView: StaticView; @@ -174,7 +172,6 @@ export class App extends React.Component { <> - <> {this.getSheet} @@ -214,12 +211,11 @@ export class App extends React.Component { export default connect( ({ application: {leftSidebarVisible, activeSheet, share}, - connections: {errors, staticView}, + connections: {staticView}, }) => ({ leftSidebarVisible, activeSheet, share: share, - errors, staticView, }), { diff --git a/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap b/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap index 297ea342c..1b4387c78 100644 --- a/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap +++ b/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap @@ -25,7 +25,6 @@ Object { "title": "MockAndroidDevice", }, ], - "errors": Array [], "selectedApp": "TestApp#Android#MockAndroidDevice#serial", "selectedDevice": Object { "deviceType": "physical", diff --git a/desktop/app/src/chrome/ErrorBar.tsx b/desktop/app/src/chrome/ErrorBar.tsx deleted file mode 100644 index 2bf69f3a2..000000000 --- a/desktop/app/src/chrome/ErrorBar.tsx +++ /dev/null @@ -1,200 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @format - */ - -import {styled, colors, Glyph} from 'flipper'; -import React, {useState, memo} from 'react'; -import {connect} from 'react-redux'; -import {FlipperError, dismissError} from '../reducers/connections'; -import {State as Store} from '../reducers/index'; -import {ErrorBlock, ButtonGroup, Button} from 'flipper'; -import {FlexColumn, FlexRow} from 'flipper'; - -type StateFromProps = { - errors: FlipperError[]; -}; - -type DispatchFromProps = { - dismissError: typeof dismissError; -}; - -type Props = DispatchFromProps & StateFromProps; - -const ErrorBar = memo(function ErrorBar(props: Props) { - const [collapsed, setCollapsed] = useState(true); - - if (!props.errors.length) { - return null; - } - - const errorCount = props.errors.reduce( - (sum, error) => sum + (error.occurrences || 1), - 0, - ); - - const urgentErrors = props.errors.filter((e) => e.urgent); - - return ( - - - {(collapsed ? urgentErrors : props.errors).map((error, index) => ( - props.dismissError(index)} - key={index} - error={error} - /> - ))} - - setCollapsed((c) => !c)} - title="Show / hide errors"> - - {collapsed && errorCount} - - - ); -}); - -export default connect( - ({connections: {errors}}) => ({ - errors, - }), - { - dismissError, - }, -)(ErrorBar); - -function ErrorTile({ - onDismiss, - error, -}: { - onDismiss: () => void; - error: FlipperError; -}) { - const [collapsed, setCollapsed] = useState(true); - return ( - - - - - {(error.details || error.error) && ( -