Dialog management cleanup

Summary:
This diff moves the dialogs

* Settings
* Plugin Manager
* Doctor
* Sign in
* Changelog

To use the imperative dialog APIs, rather then organising them through reducers which adds a lot of indirection which isn't really needed but hard to follow.

Reviewed By: passy

Differential Revision: D30192002

fbshipit-source-id: ba38b2e700da3e442653786448fcbf85074981ad
This commit is contained in:
Michel Weststrate
2021-10-06 09:08:47 -07:00
committed by Facebook GitHub Bot
parent 89b193b438
commit 9e5575cf69
14 changed files with 112 additions and 105 deletions

View File

@@ -16,12 +16,6 @@ import {
startFileExport,
startLinkExport,
} from './utils/exportData';
import {
setActiveSheet,
ACTIVE_SHEET_PLUGINS,
ACTIVE_SHEET_SETTINGS,
ACTIVE_SHEET_CHANGELOG,
} from './reducers/application';
import {setStaticView} from './reducers/connections';
import {Store} from './reducers/';
import electron, {MenuItemConstructorOptions} from 'electron';
@@ -33,11 +27,16 @@ import {
_buildInMenuEntries,
_wrapInteractionHandler,
getFlipperLib,
Dialog,
} from 'flipper-plugin';
import {StyleGuide} from './sandy-chrome/StyleGuide';
import {showEmulatorLauncher} from './sandy-chrome/appinspect/LaunchEmulator';
import {webFrame} from 'electron';
import {openDeeplinkDialog} from './deeplink';
import React from 'react';
import ChangelogSheet from './chrome/ChangelogSheet';
import PluginManager from './chrome/plugin-manager/PluginManager';
import SettingsSheet from './chrome/SettingsSheet';
export type DefaultKeyboardAction = keyof typeof _buildInMenuEntries;
export type TopLevelMenu = 'Edit' | 'View' | 'Window' | 'Help';
@@ -253,7 +252,11 @@ function getTemplate(
{
label: 'Preferences',
accelerator: 'Cmd+,',
click: () => store.dispatch(setActiveSheet(ACTIVE_SHEET_SETTINGS)),
click: () => {
Dialog.showModal((onHide) => (
<SettingsSheet platform={process.platform} onHide={onHide} />
));
},
},
{
label: 'Import Flipper File...',
@@ -343,9 +346,12 @@ function getTemplate(
{
label: 'Manage Plugins...',
click: function () {
store.dispatch(setActiveSheet(ACTIVE_SHEET_PLUGINS));
Dialog.showModal((onHide) => <PluginManager onHide={onHide} />);
},
},
{
type: 'separator',
},
{
label: 'Flipper style guide',
click() {
@@ -404,7 +410,7 @@ function getTemplate(
{
label: 'Changelog',
click() {
store.dispatch(setActiveSheet(ACTIVE_SHEET_CHANGELOG));
Dialog.showModal((onHide) => <ChangelogSheet onHide={onHide} />);
},
},
];

View File

@@ -47,7 +47,6 @@ type DispatchFromProps = {
type Props = OwnProps & StateFromProps & DispatchFromProps;
const Container = styled(FlexColumn)({
width: 700,
maxHeight: 700,
padding: 8,
});

View File

@@ -9,26 +9,15 @@
import React, {useCallback} from 'react';
import ShareSheetExportUrl from './ShareSheetExportUrl';
import SignInSheet from './fb-stubs/SignInSheet';
import ExportDataPluginSheet from './ExportDataPluginSheet';
import ShareSheetExportFile from './ShareSheetExportFile';
import Sheet from './Sheet';
import {
ACTIVE_SHEET_PLUGINS,
ACTIVE_SHEET_SHARE_DATA,
ACTIVE_SHEET_SIGN_IN,
ACTIVE_SHEET_SETTINGS,
ACTIVE_SHEET_DOCTOR,
ACTIVE_SHEET_SHARE_DATA_IN_FILE,
ACTIVE_SHEET_SELECT_PLUGINS_TO_EXPORT,
ACTIVE_SHEET_CHANGELOG,
ACTIVE_SHEET_CHANGELOG_RECENT_ONLY,
} from '../reducers/application';
import {Logger} from '../fb-interfaces/Logger';
import PluginManager from './plugin-manager/PluginManager';
import SettingsSheet from './SettingsSheet';
import DoctorSheet from './DoctorSheet';
import ChangelogSheet from './ChangelogSheet';
import {useStore} from '../utils/useStore';
export function SheetRenderer({logger}: {logger: Logger}) {
@@ -39,18 +28,6 @@ export function SheetRenderer({logger}: {logger: Logger}) {
const renderSheet = useCallback(
(onHide: () => any) => {
switch (activeSheet) {
case ACTIVE_SHEET_PLUGINS:
return <PluginManager onHide={onHide} />;
case ACTIVE_SHEET_SIGN_IN:
return <SignInSheet onHide={onHide} />;
case ACTIVE_SHEET_SETTINGS:
return <SettingsSheet platform={process.platform} onHide={onHide} />;
case ACTIVE_SHEET_DOCTOR:
return <DoctorSheet onHide={onHide} />;
case ACTIVE_SHEET_CHANGELOG:
return <ChangelogSheet onHide={onHide} />;
case ACTIVE_SHEET_CHANGELOG_RECENT_ONLY:
return <ChangelogSheet onHide={onHide} recent />;
case ACTIVE_SHEET_SELECT_PLUGINS_TO_EXPORT:
return <ExportDataPluginSheet onHide={onHide} />;
case ACTIVE_SHEET_SHARE_DATA:

View File

@@ -7,6 +7,8 @@
* @format
*/
export default function SignInSheetFunc(_: {onHide: () => any}) {
return null;
export async function showLoginDialog(
_initialToken: string = '',
): Promise<boolean> {
return false;
}

View File

@@ -7,11 +7,6 @@
* @format
*/
import {
ACTIVE_SHEET_SIGN_IN,
setActiveSheet,
setPastedToken,
} from './reducers/application';
import {Group, SUPPORTED_GROUPS} from './reducers/supportForm';
import {Logger} from './fb-interfaces/Logger';
import {Store} from './reducers/index';
@@ -20,6 +15,7 @@ import {selectPlugin, getAllClients} from './reducers/connections';
import {Dialog} from 'flipper-plugin';
import {handleOpenPluginDeeplink} from './dispatcher/handleOpenPluginDeeplink';
import {message} from 'antd';
import {showLoginDialog} from './chrome/fb-stubs/SignInSheet';
import {track} from './deeplinkTracking';
const UNKNOWN = 'Unknown deeplink';
@@ -81,10 +77,7 @@ export async function handleDeeplink(
throw unknownError();
} else if (uri.pathname.match(/^\/*login\/*$/)) {
const token = uri.searchParams.get('token');
store.dispatch(setPastedToken(token ?? undefined));
if (store.getState().application.activeSheet !== ACTIVE_SHEET_SIGN_IN) {
store.dispatch(setActiveSheet(ACTIVE_SHEET_SIGN_IN));
}
showLoginDialog(token ?? '');
return;
}
const match = uriComponents(query);

View File

@@ -13,7 +13,6 @@ import {getUser} from '../fb-stubs/user';
import {State, Store} from '../reducers/index';
import {checkForUpdate} from '../fb-stubs/checkForUpdate';
import {getAppVersion} from '../utils/info';
import {ACTIVE_SHEET_SIGN_IN, setActiveSheet} from '../reducers/application';
import {UserNotSignedInError} from '../utils/errors';
import {selectPlugin, setPluginEnabled} from '../reducers/connections';
import {getUpdateAvailableMessage} from '../chrome/UpdateIndicator';
@@ -29,6 +28,7 @@ import Client from '../Client';
import {RocketOutlined} from '@ant-design/icons';
import {showEmulatorLauncher} from '../sandy-chrome/appinspect/LaunchEmulator';
import {getAllClients} from '../reducers/connections';
import {showLoginDialog} from '../chrome/fb-stubs/SignInSheet';
import {DeeplinkInteraction, OpenPluginParams} from '../deeplinkTracking';
export function parseOpenPluginParams(query: string): OpenPluginParams {
@@ -233,7 +233,7 @@ async function showPleaseLoginDialog(
return false;
}
store.dispatch(setActiveSheet(ACTIVE_SHEET_SIGN_IN));
await showLoginDialog();
// wait until login succeeded
await waitForLogin(store);
return true;

View File

@@ -14,7 +14,6 @@ import {v1 as uuidv1} from 'uuid';
import {ReactElement} from 'react';
import CancellableExportStatus from '../chrome/CancellableExportStatus';
import {Actions} from './';
import produce from 'immer';
export const ACTIVE_SHEET_PLUGINS: 'PLUGINS' = 'PLUGINS';
export const ACTIVE_SHEET_SELECT_PLUGINS_TO_EXPORT: 'SELECT_PLUGINS_TO_EXPORT' =
'SELECT_PLUGINS_TO_EXPORT';
@@ -31,16 +30,13 @@ export const ACTIVE_SHEET_CHANGELOG = 'ACTIVE_SHEET_CHANGELOG';
export const ACTIVE_SHEET_CHANGELOG_RECENT_ONLY =
'ACTIVE_SHEET_CHANGELOG_RECENT_ONLY';
/**
* @deprecated this is a weird mechanism, and using imperative dialogs will be simpler
*/
export type ActiveSheet =
| typeof ACTIVE_SHEET_PLUGINS
| typeof ACTIVE_SHEET_SHARE_DATA
| typeof ACTIVE_SHEET_SIGN_IN
| typeof ACTIVE_SHEET_SETTINGS
| typeof ACTIVE_SHEET_DOCTOR
| typeof ACTIVE_SHEET_SHARE_DATA_IN_FILE
| typeof ACTIVE_SHEET_SELECT_PLUGINS_TO_EXPORT
| typeof ACTIVE_SHEET_CHANGELOG
| typeof ACTIVE_SHEET_CHANGELOG_RECENT_ONLY
| null;
export type LauncherMsg = {
@@ -84,7 +80,6 @@ export type State = {
altServerPorts: ServerPorts;
launcherMsg: LauncherMsg;
statusMessages: Array<string>;
pastedToken?: string;
};
type BooleanActionType =
@@ -152,10 +147,6 @@ export type Action =
| {
type: 'REMOVE_STATUS_MSG';
payload: {msg: string; sender: string};
}
| {
type: 'SET_PASTED_TOKEN';
payload?: string;
};
export const initialState: () => State = () => ({
@@ -294,10 +285,6 @@ export default function reducer(
return {...state, statusMessages};
}
return state;
} else if (action.type === 'SET_PASTED_TOKEN') {
return produce(state, (draft) => {
draft.pastedToken = action.payload;
});
} else {
return state;
}
@@ -371,8 +358,3 @@ export const removeStatusMessage = (payload: StatusMessageType): Action => ({
type: 'REMOVE_STATUS_MSG',
payload,
});
export const setPastedToken = (pastedToken?: string): Action => ({
type: 'SET_PASTED_TOKEN',
payload: pastedToken,
});

View File

@@ -24,13 +24,10 @@ import {
import {SidebarLeft, SidebarRight} from './SandyIcons';
import {useDispatch, useStore} from '../utils/useStore';
import {
ACTIVE_SHEET_PLUGINS,
ACTIVE_SHEET_SIGN_IN,
setActiveSheet,
toggleLeftSidebarVisible,
toggleRightSidebarVisible,
} from '../reducers/application';
import {theme, Layout, withTrackingScope} from 'flipper-plugin';
import {theme, Layout, withTrackingScope, Dialog} from 'flipper-plugin';
import SetupDoctorScreen, {checkHasNewProblem} from './SetupDoctorScreen';
import SettingsSheet from '../chrome/SettingsSheet';
import WelcomeScreen from './WelcomeScreen';
@@ -51,6 +48,8 @@ import isProduction from '../utils/isProduction';
import NetworkGraph from '../chrome/NetworkGraph';
import FpsGraph from '../chrome/FpsGraph';
import UpdateIndicator from '../chrome/UpdateIndicator';
import PluginManager from '../chrome/plugin-manager/PluginManager';
import {showLoginDialog} from '../chrome/fb-stubs/SignInSheet';
const LeftRailButtonElem = styled(Button)<{kind?: 'small'}>(({kind}) => ({
width: kind === 'small' ? 32 : 36,
@@ -119,7 +118,6 @@ export const LeftRail = withTrackingScope(function LeftRail({
toplevelSelection,
setToplevelSelection,
}: ToplevelProps) {
const dispatch = useDispatch();
return (
<Layout.Container borderRight padv={12} width={48}>
<Layout.Bottom style={{overflow: 'visible'}}>
@@ -136,7 +134,7 @@ export const LeftRail = withTrackingScope(function LeftRail({
icon={<AppstoreOutlined />}
title="Plugin Manager"
onClick={() => {
dispatch(setActiveSheet(ACTIVE_SHEET_PLUGINS));
Dialog.showModal((onHide) => <PluginManager onHide={onHide} />);
}}
/>
<NotificationButton
@@ -362,9 +360,6 @@ function LoginButton() {
const user = useStore((state) => state.user);
const login = (user?.id ?? null) !== null;
const profileUrl = user?.profile_picture?.uri;
const showLogin = useCallback(() => {
dispatch(setActiveSheet(ACTIVE_SHEET_SIGN_IN));
}, [dispatch]);
const [showLogout, setShowLogout] = useState(false);
const onHandleVisibleChange = useCallback(
(visible) => setShowLogout(visible),
@@ -398,7 +393,7 @@ function LoginButton() {
<LeftRailButton
icon={<LoginOutlined />}
title="Log In"
onClick={showLogin}
onClick={() => showLoginDialog()}
/>
</>
);

View File

@@ -8,7 +8,14 @@
*/
import React, {useEffect, useState, useCallback} from 'react';
import {TrackingScope, useLogger, _Sidebar, Layout} from 'flipper-plugin';
import {
TrackingScope,
useLogger,
_Sidebar,
Layout,
Dialog,
_PortalsManager,
} from 'flipper-plugin';
import {Link, styled} from '../ui';
import {theme} from 'flipper-plugin';
import {ipcRenderer} from 'electron';
@@ -18,17 +25,13 @@ import {LeftRail} from './LeftRail';
import {useStore, useDispatch} from '../utils/useStore';
import {FlipperDevTools} from '../chrome/FlipperDevTools';
import {setStaticView} from '../reducers/connections';
import {
ACTIVE_SHEET_CHANGELOG_RECENT_ONLY,
setActiveSheet,
toggleLeftSidebarVisible,
} from '../reducers/application';
import {toggleLeftSidebarVisible} from '../reducers/application';
import {AppInspect} from './appinspect/AppInspect';
import PluginContainer from '../PluginContainer';
import {ContentContainer} from './ContentContainer';
import {Notification} from './notification/Notification';
import {SheetRenderer} from '../chrome/SheetRenderer';
import {hasNewChangesToShow} from '../chrome/ChangelogSheet';
import ChangelogSheet, {hasNewChangesToShow} from '../chrome/ChangelogSheet';
import {getVersionString} from '../utils/versionString';
import config from '../fb-stubs/config';
import {WelcomeScreenStaticView} from './WelcomeScreen';
@@ -94,7 +97,7 @@ export function SandyApp() {
registerStartupTime(logger);
if (hasNewChangesToShow(window.localStorage)) {
dispatch(setActiveSheet(ACTIVE_SHEET_CHANGELOG_RECENT_ONLY));
Dialog.showModal((onHide) => <ChangelogSheet onHide={onHide} recent />);
}
// don't warn about logger, even with a new logger we don't want to re-register
// eslint-disable-next-line
@@ -122,7 +125,9 @@ export function SandyApp() {
});
}
})
.catch((e) => console.error('isEmployee check failed:', e));
.catch((e) => {
console.warn('Failed to check if user is employee', e);
});
}
}, []);
@@ -134,8 +139,7 @@ export function SandyApp() {
) : null;
return (
<Layout.Top>
<SheetRenderer logger={logger} />
<Layout.Container grow>
<Layout.Left>
<Layout.Horizontal>
<LeftRail
@@ -175,7 +179,9 @@ export function SandyApp() {
{outOfContentsContainer}
</MainContainer>
</Layout.Left>
</Layout.Top>
<SheetRenderer logger={logger} />
<_PortalsManager />
</Layout.Container>
);
}

View File

@@ -17,11 +17,7 @@ import {CertificateExchangeMedium} from './utils/CertificateProvider';
import {isLoggedIn} from '../fb-stubs/user';
import React from 'react';
import {Typography} from 'antd';
import {
ACTIVE_SHEET_SIGN_IN,
ServerPorts,
setActiveSheet,
} from '../reducers/application';
import {ServerPorts} from '../reducers/application';
import {AndroidDeviceManager} from './devices/android/androidDeviceManager';
import {IOSDeviceManager} from './devices/ios/iOSDeviceManager';
import metroDevice from './devices/metro/metroDeviceManager';
@@ -35,6 +31,7 @@ import {
import {ServerDevice} from './devices/ServerDevice';
import {Base64} from 'js-base64';
import MetroDevice from './devices/metro/MetroDevice';
import {showLoginDialog} from '../chrome/fb-stubs/SignInSheet';
export interface FlipperServerConfig {
enableAndroid: boolean;
@@ -135,11 +132,9 @@ export class FlipperServerImpl implements FlipperServer {
<>
and{' '}
<Typography.Link
onClick={() =>
this.store.dispatch(
setActiveSheet(ACTIVE_SHEET_SIGN_IN),
)
}>
onClick={() => {
showLoginDialog();
}}>
log in to Facebook Intern
</Typography.Link>
</>

View File

@@ -85,6 +85,7 @@ test('Correct top level API exposed', () => {
"DeviceLogEntry",
"DeviceLogListener",
"DevicePluginClient",
"DialogResult",
"Draft",
"ElementAttribute",
"ElementData",

View File

@@ -61,7 +61,7 @@ export {Toolbar} from './ui/Toolbar';
export {MasterDetail} from './ui/MasterDetail';
export {CodeBlock} from './ui/CodeBlock';
export {renderReactRoot} from './utils/renderReactRoot';
export {renderReactRoot, _PortalsManager} from './utils/renderReactRoot';
export {
Tracked,
TrackingScope,
@@ -115,7 +115,7 @@ export {
} from './ui/data-inspector/DataDescription';
export {MarkerTimeline} from './ui/MarkerTimeline';
export {DataInspector} from './ui/data-inspector/DataInspector';
export {Dialog} from './ui/Dialog';
export {Dialog, DialogResult} from './ui/Dialog';
export {
ElementsInspector,
Element as ElementsInspectorElement,

View File

@@ -13,7 +13,8 @@ import React from 'react';
import {renderReactRoot} from '../utils/renderReactRoot';
import {Layout} from './Layout';
import {Spinner} from './Spinner';
type DialogResult<T> = Promise<false | T> & {close: () => void};
export type DialogResult<T> = Promise<false | T> & {close: () => void};
type BaseDialogOptions = {
title: string;
@@ -104,6 +105,36 @@ export const Dialog = {
);
},
/**
* Shows an item in the modal stack, but without providing any further UI, like .show does.
*/
showModal<T = void>(
fn: (hide: (result?: T) => void) => React.ReactElement,
): DialogResult<T> {
let cancel: () => void;
return Object.assign(
new Promise<false | T>((resolve) => {
renderReactRoot((hide) => {
cancel = () => {
hide();
resolve(false);
};
return fn((result?: T) => {
hide();
resolve(result ?? false);
});
});
}),
{
close() {
cancel();
},
},
);
},
confirm({
message,
onConfirm,

View File

@@ -7,8 +7,9 @@
* @format
*/
import React from 'react';
import {render, unmountComponentAtNode} from 'react-dom';
import {createState, useValue} from '../state/atom';
import React, {ReactPortal} from 'react';
import {createPortal, unmountComponentAtNode} from 'react-dom';
/**
* This utility creates a fresh react render hook, which is great to render elements imperatively, like opening dialogs.
@@ -20,9 +21,28 @@ export function renderReactRoot(
// TODO: find a way to make this visible in unit tests as well
const div = document.body.appendChild(document.createElement('div'));
const unmount = () => {
portals.update((draft) => {
draft.delete(id);
});
unmountComponentAtNode(div);
div.remove();
};
render(handler(unmount), div);
const id = ++portalId;
const portal = createPortal(handler(unmount), div);
portals.update((draft) => {
draft.set(id, portal);
});
return unmount;
}
let portalId = 0;
const portals = createState(new Map<number, ReactPortal>());
/**
* This is a dummy component, that just makes sure react roots are managed within a certain node in the main React tree, so that context etc is available.
*/
export function _PortalsManager() {
const portalElements = useValue(portals);
return <>{Array.from(portalElements).map(([_id, portal]) => portal)}</>;
}