From 9f7be13e39a0a274d4969a8d825731f892828d99 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 18 Nov 2019 02:18:04 -0800 Subject: [PATCH] Rework sidebar selection Summary: - Make sure newly connecting apps are automatically selected - Improved the sidebar UI by using more consistent, spacious styling, and giving some more attention to error states Reviewed By: passy Differential Revision: D18505636 fbshipit-source-id: 18b2c8e78be13aabb3a54c60553f6b0d1e613b27 --- headless/index.tsx | 3 + src/PluginContainer.tsx | 2 +- src/chrome/MainSidebar.tsx | 200 ++++++++++++------------ src/devices/BaseDevice.tsx | 12 ++ src/dispatcher/androidDevice.tsx | 10 +- src/dispatcher/desktopDevice.tsx | 1 + src/dispatcher/iOSDevice.tsx | 1 + src/plugin.tsx | 7 + src/reducers/connections.tsx | 255 +++++++++++++++++-------------- src/reducers/index.tsx | 1 - src/utils/exportData.tsx | 3 +- src/utils/icons.js | 3 +- 12 files changed, 284 insertions(+), 214 deletions(-) diff --git a/headless/index.tsx b/headless/index.tsx index e2b56a9f8..2c078201a 100644 --- a/headless/index.tsx +++ b/headless/index.tsx @@ -314,6 +314,9 @@ async function startFlipper(userArguments: UserArguments) { const ports = store.getState().application.serverPorts; matchedDevice.reverse([ports.secure, ports.insecure]); } + matchedDevice.loadDevicePlugins( + store.getState().plugins.devicePlugins, + ); store.dispatch({ type: 'REGISTER_DEVICE', payload: matchedDevice, diff --git a/src/PluginContainer.tsx b/src/PluginContainer.tsx index 672f9a424..548ba8106 100644 --- a/src/PluginContainer.tsx +++ b/src/PluginContainer.tsx @@ -224,7 +224,7 @@ export default connect( deepLinkPayload, pluginKey, isArchivedDevice, - selectedApp, + selectedApp: selectedApp || null, }; return s; }, diff --git a/src/chrome/MainSidebar.tsx b/src/chrome/MainSidebar.tsx index aa8d5f217..5292b228a 100644 --- a/src/chrome/MainSidebar.tsx +++ b/src/chrome/MainSidebar.tsx @@ -11,7 +11,7 @@ import config from '../fb-stubs/config'; import BaseDevice from '../devices/BaseDevice'; import Client from '../Client'; import {UninitializedClient} from '../UninitializedClient'; -import {FlipperBasePlugin} from '../plugin'; +import {FlipperBasePlugin, sortPluginsByName} from '../plugin'; import {PluginNotification} from '../reducers/notifications'; import {ActiveSheet, ACTIVE_SHEET_PLUGINS} from '../reducers/application'; import {State as Store} from '../reducers'; @@ -31,6 +31,8 @@ import { Button, StarButton, ArchivedDevice, + Heading, + Spacer, } from 'flipper'; import React, {Component, PureComponent, Fragment} from 'react'; import NotificationsHub from '../NotificationsHub'; @@ -40,6 +42,8 @@ import { StaticView, setStaticView, selectClient, + getAvailableClients, + getClientById, } from '../reducers/connections'; import {setActiveSheet} from '../reducers/application'; import UserAccount from './UserAccount'; @@ -56,7 +60,7 @@ const ListItem = styled('div')(({active}: {active?: boolean}) => ({ paddingLeft: 10, display: 'flex', alignItems: 'center', - marginBottom: 2, + marginBottom: 6, flexShrink: 0, backgroundColor: active ? colors.macOSTitleBarIconSelected : 'none', color: active ? colors.white : colors.macOSSidebarSectionItem, @@ -83,7 +87,7 @@ const SidebarButton = styled(Button)(({small}: {small?: boolean}) => ({ const PluginShape = styled(FlexBox)( ({backgroundColor}: {backgroundColor?: BackgroundColorProperty}) => ({ - marginRight: 5, + marginRight: 8, backgroundColor, borderRadius: 3, flexShrink: 0, @@ -91,6 +95,7 @@ const PluginShape = styled(FlexBox)( height: 18, justifyContent: 'center', alignItems: 'center', + top: '-1px', }), ); @@ -225,7 +230,6 @@ type StateFromProps = { selectedApp: string | null | undefined; userStarredPlugins: Store['connections']['userStarredPlugins']; clients: Array; - selectedClient: string; uninitializedClients: Array<{ client: UninitializedClient; deviceId?: string; @@ -273,30 +277,18 @@ class MainSidebar extends PureComponent { render() { const { selectedDevice, - selectedClient, selectClient, selectedPlugin, + selectedApp, staticView, selectPlugin, setStaticView, windowIsFocused, numNotifications, + uninitializedClients, } = this.props; - let {clients, uninitializedClients} = this.props; - clients = clients - .filter( - (client: Client) => - (selectedDevice && - selectedDevice.supportsOS(client.query.os) && - client.query.device_id === selectedDevice.serial) || - // Old android sdk versions don't know their device_id - // Display their plugins under all selected devices until they die out - client.query.device_id === 'unknown', - ) - .sort((a, b) => (a.query.app || '').localeCompare(b.query.app)); - const client: Client | undefined = clients.find( - client => client.query.app === selectedClient, - ); + const clients = getAvailableClients(selectedDevice, this.props.clients); + const client: Client | undefined = getClientById(clients, selectedApp); return ( { process.platform === 'darwin' && windowIsFocused ? 'transparent' : '' }> - {selectedDevice && ( + {selectedDevice ? ( <> {selectedDevice.title} {this.showArchivedDeviceDetails(selectedDevice)} - - )} - {selectedDevice && - Array.from(this.props.devicePlugins.values()) - .filter(plugin => plugin.supportsDevice(selectedDevice)) - .sort(sortPluginsByName) - .map((plugin: typeof FlipperDevicePlugin) => ( - - selectPlugin({ - selectedPlugin: plugin.id, - selectedApp: null, - deepLinkPayload: null, - }) - } - plugin={plugin} - /> - ))} - - ({ - checked: client === c, - label: c.query.app, - type: 'checkbox', - click: () => selectClient(c.query.app), - }))}> - {clients.length === 0 ? ( - '(No clients connected)' - ) : !client ? ( - '(Select client)' - ) : ( - <> - {client.query.app} - {clients.length > 1 && ( - + {selectedDevice.devicePlugins.map(pluginName => { + const plugin = this.props.devicePlugins.get(pluginName)!; + return ( + + selectPlugin({ + selectedPlugin: plugin.id, + selectedApp: null, + deepLinkPayload: null, + }) + } + plugin={plugin} + /> + ); + })} + + ({ + checked: client === c, + label: c.query.app, + type: 'checkbox', + click: () => selectClient(c.id), + }))}> + {clients.length === 0 ? ( + <> + + No clients connected + + ) : !client ? ( + 'Select client' + ) : ( + <> + {client.query.app} + + )} - - )} - - - {this.renderClientPlugins(client)} - {uninitializedClients.map(entry => ( - - {entry.client.appName} - {entry.errorMessage ? ( - - ) : ( - - )} + + + {this.renderClientPlugins(client)} + {uninitializedClients.map(entry => ( + + {entry.client.appName} + {entry.errorMessage ? ( + + ) : ( + + )} + + ))} + + ) : ( + + + + Select a device to get started - ))} + )} {!GK.get('flipper_disable_notifications') && ( { this.props.userStarredPlugins, true, ); + const showAllPlugins = + this.state.showAllPlugins || + favoritePlugins.length === 0 || + // If the plugin is part of the hidden section, make sure sidebar is expanded + (client.plugins.includes(this.props.selectedPlugin!) && + !favoritePlugins.find( + plugin => plugin.id === this.props.selectedPlugin, + )); return ( <> {favoritePlugins.length === 0 ? ( -
- Star some plugins! -
+
+ star your favorite plugins!
) : ( @@ -562,12 +583,10 @@ class MainSidebar extends PureComponent { showAllPlugins: !state.showAllPlugins, })) }> - {this.state.showAllPlugins ? 'Show less' : 'Show more'} + {showAllPlugins ? 'Show less' : 'Show more'} { overflow: 'auto', height: 'auto', }}> - {this.state.showAllPlugins || favoritePlugins.length === 0 + {showAllPlugins ? this.renderPluginsByCategory( client, getFavoritePlugins( @@ -633,13 +652,6 @@ function groupPluginsByCategory(plugins: FlipperPlugins): PluginsByCategory { return res; } -function sortPluginsByName( - a: typeof FlipperBasePlugin, - b: typeof FlipperBasePlugin, -): number { - return (a.title || a.id) > (b.title || b.id) ? 1 : -1; -} - export default connect( ({ application: {windowIsFocused}, @@ -647,7 +659,6 @@ export default connect( selectedDevice, selectedPlugin, selectedApp, - selectedClient, userStarredPlugins, clients, uninitializedClients, @@ -664,7 +675,6 @@ export default connect( })(), windowIsFocused, selectedDevice, - selectedClient, staticView, selectedPlugin, selectedApp, diff --git a/src/devices/BaseDevice.tsx b/src/devices/BaseDevice.tsx index 4f8c5dbf8..a767495ca 100644 --- a/src/devices/BaseDevice.tsx +++ b/src/devices/BaseDevice.tsx @@ -8,6 +8,8 @@ */ import stream from 'stream'; +import {FlipperDevicePlugin} from 'flipper'; +import {sortPluginsByName} from '../plugin'; export type LogLevel = | 'unknown' @@ -81,6 +83,9 @@ export default class BaseDevice { // if imported, stores the original source location source = ''; + // sorted list of supported device plugins + devicePlugins!: string[]; + supportsOS(os: OS) { return os.toLowerCase() === this.os.toLowerCase(); } @@ -164,4 +169,11 @@ export default class BaseDevice { async stopScreenCapture(): Promise { return null; } + + loadDevicePlugins(devicePlugins?: Map) { + this.devicePlugins = Array.from(devicePlugins ? devicePlugins.values() : []) + .filter(plugin => plugin.supportsDevice(this)) + .sort(sortPluginsByName) + .map(plugin => plugin.id); + } } diff --git a/src/dispatcher/androidDevice.tsx b/src/dispatcher/androidDevice.tsx index 3d98786c2..7b6403df1 100644 --- a/src/dispatcher/androidDevice.tsx +++ b/src/dispatcher/androidDevice.tsx @@ -186,6 +186,7 @@ export default (store: Store, logger: Logger) => { payload: new Set(reconnectedDevices), }); + androidDevice.loadDevicePlugins(store.getState().plugins.devicePlugins); store.dispatch({ type: 'REGISTER_DEVICE', payload: androidDevice, @@ -223,12 +224,13 @@ export default (store: Store, logger: Logger) => { payload: new Set(deviceIds), }); - archivedDevices.forEach((payload: BaseDevice) => + archivedDevices.forEach((device: BaseDevice) => { + device.loadDevicePlugins(store.getState().plugins.devicePlugins); store.dispatch({ type: 'REGISTER_DEVICE', - payload, - }), - ); + payload: device, + }); + }); } watchAndroidDevices(); diff --git a/src/dispatcher/desktopDevice.tsx b/src/dispatcher/desktopDevice.tsx index 268445ad6..407ab84ad 100644 --- a/src/dispatcher/desktopDevice.tsx +++ b/src/dispatcher/desktopDevice.tsx @@ -22,6 +22,7 @@ export default (store: Store, logger: Logger) => { } else { return; } + device.loadDevicePlugins(store.getState().plugins.devicePlugins); store.dispatch({ type: 'REGISTER_DEVICE', payload: device, diff --git a/src/dispatcher/iOSDevice.tsx b/src/dispatcher/iOSDevice.tsx index e36a073c6..0b7565303 100644 --- a/src/dispatcher/iOSDevice.tsx +++ b/src/dispatcher/iOSDevice.tsx @@ -91,6 +91,7 @@ async function queryDevices(store: Store, logger: Logger): Promise { serial: udid, }); const iOSDevice = new IOSDevice(udid, type, name); + iOSDevice.loadDevicePlugins(store.getState().plugins.devicePlugins); store.dispatch({ type: 'REGISTER_DEVICE', payload: iOSDevice, diff --git a/src/plugin.tsx b/src/plugin.tsx index dcc4e442d..426fb0573 100644 --- a/src/plugin.tsx +++ b/src/plugin.tsx @@ -265,3 +265,10 @@ export class FlipperPlugin< this.init(); } } + +export function sortPluginsByName( + a: typeof FlipperBasePlugin, + b: typeof FlipperBasePlugin, +): number { + return (a.title || a.id) > (b.title || b.id) ? 1 : -1; +} diff --git a/src/reducers/connections.tsx b/src/reducers/connections.tsx index f450a547b..7370137f6 100644 --- a/src/reducers/connections.tsx +++ b/src/reducers/connections.tsx @@ -40,16 +40,13 @@ export type State = { androidEmulators: Array; selectedDevice: null | BaseDevice; selectedPlugin: null | string; - selectedApp: null | string; + selectedApp: null | string | undefined; userPreferredDevice: null | string; userPreferredPlugin: null | string; userPreferredApp: null | string; userStarredPlugins: {[key: string]: Array}; errors: FlipperError[]; clients: Array; - // refers to the client that is selected in the main side bar, not to be confused with - // selectedApp, which represents the app of the currently active plugin! - selectedClient: string; uninitializedClients: Array<{ client: UninitializedClient; deviceId?: string; @@ -154,7 +151,6 @@ const INITAL_STATE: State = { userStarredPlugins: {}, errors: [], clients: [], - selectedClient: '', uninitializedClients: [], deepLinkPayload: null, staticView: WelcomeScreen, @@ -173,14 +169,13 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { } case 'SELECT_DEVICE': { const {payload} = action; - return { + return updateSelection({ ...state, - selectedApp: null, - staticView: null, - selectedPlugin: DEFAULT_PLUGIN, selectedDevice: payload, - userPreferredDevice: payload.title, - }; + userPreferredDevice: payload + ? payload.title + : state.userPreferredDevice, + }); } case 'REGISTER_ANDROID_EMULATORS': { const {payload} = action; @@ -191,76 +186,27 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { } case 'REGISTER_DEVICE': { const {payload} = action; - const devices = state.devices.concat(payload); - let {selectedDevice, selectedPlugin} = state; - let staticView: StaticView = state.staticView; - // select the default plugin - let selection: Partial = { - selectedApp: null, - selectedPlugin: DEFAULT_PLUGIN, - }; - const canBeDefaultDevice = !DEFAULT_DEVICE_BLACKLIST.some( - blacklistedDevice => payload instanceof blacklistedDevice, - ); - - if (!selectedDevice && canBeDefaultDevice) { - selectedDevice = payload; - staticView = null; - if (selectedPlugin) { - // We already had a plugin selected, but no device. This is happening - // when the Client connected before the Device. - selection = {}; - } - } else if (payload.title === state.userPreferredDevice) { - selectedDevice = payload; - staticView = null; - } else { - // We didn't select the newly connected device, so we don't want to - // change the plugin. - selection = {}; - } - - return { + return updateSelection({ ...state, - devices, - // select device if none was selected before - selectedDevice, - ...selection, - staticView, - }; + devices: state.devices.concat(payload), + }); } + case 'UNREGISTER_DEVICES': { const {payload} = action; - const {selectedDevice} = state; - let selectedDeviceWasRemoved = false; const devices = state.devices.filter((device: BaseDevice) => { if (payload.has(device.serial)) { - if (selectedDevice === device) { - // removed device is the selected - selectedDeviceWasRemoved = true; - } return false; } else { return true; } }); - let selection = {}; - if (selectedDeviceWasRemoved) { - selection = { - selectedDevice: devices[devices.length - 1] || null, - staticView: selectedDevice != null ? null : WelcomeScreen, - selectedApp: null, - selectedPlugin: DEFAULT_PLUGIN, - }; - } - - return { + return updateSelection({ ...state, devices, - ...selection, - }; + }); } case 'SELECT_PLUGIN': { const {payload} = action; @@ -269,15 +215,14 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { performance.mark(`activePlugin-${selectedPlugin}`); } - const userPreferredApp = selectedApp || state.userPreferredApp; - return { + return updateSelection({ ...state, - ...payload, - staticView: null, - userPreferredApp: userPreferredApp, - userPreferredPlugin: selectedPlugin, - }; + selectedApp, + selectedPlugin, + userPreferredPlugin: selectedPlugin || state.userPreferredPlugin, + }); } + case 'STAR_PLUGIN': { const {selectedPlugin, selectedApp} = action.payload; const starredPluginsForApp = [ @@ -302,22 +247,11 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { const {payload} = action; return {...state, userPreferredPlugin: payload}; } + case 'NEW_CLIENT': { const {payload} = action; - const {userPreferredApp, userPreferredPlugin} = state; - let {selectedApp, selectedPlugin} = state; - if ( - userPreferredApp && - userPreferredPlugin && - payload.id === userPreferredApp && - payload.plugins.includes(userPreferredPlugin) - ) { - // user preferred client did reconnect, so let's select it - selectedApp = userPreferredApp; - selectedPlugin = userPreferredPlugin; - } - return { + return updateSelection({ ...state, clients: state.clients.concat(payload), uninitializedClients: state.uninitializedClients.filter(c => { @@ -326,10 +260,18 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { c.client.appName !== payload.query.app ); }), - selectedApp, - selectedPlugin, - }; + }); } + + case 'SELECT_CLIENT': { + const {payload} = action; + return updateSelection({ + ...state, + selectedApp: payload, + userPreferredApp: payload || state.userPreferredApp, + }); + } + case 'NEW_CLIENT_SANITY_CHECK': { const {payload} = action; // Check for clients initialised when there is no matching device @@ -346,26 +288,19 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { ); } } - return state; } + case 'CLIENT_REMOVED': { const {payload} = action; - - const selected: Partial = {}; - if (state.selectedApp === payload) { - selected.selectedApp = null; - selected.selectedPlugin = DEFAULT_PLUGIN; - } - - return { + return updateSelection({ ...state, - ...selected, clients: state.clients.filter( (client: Client) => client.id !== payload, ), - }; + }); } + case 'PREFER_DEVICE': { const {payload: userPreferredDevice} = action; return {...state, userPreferredDevice}; @@ -433,12 +368,6 @@ const reducer = (state: State = INITAL_STATE, action: Actions): State => { errors, }; } - case 'SELECT_CLIENT': { - return { - ...state, - selectedClient: action.payload, - }; - } default: return state; } @@ -523,11 +452,6 @@ export const starPlugin = (payload: { payload, }); -export const userPreferredPlugin = (payload: string): Action => ({ - type: 'SELECT_USER_PREFERRED_PLUGIN', - payload, -}); - export const dismissError = (index: number): Action => ({ type: 'DISMISS_ERROR', payload: index, @@ -537,3 +461,112 @@ export const selectClient = (clientId: string): Action => ({ type: 'SELECT_CLIENT', payload: clientId, }); + +export function getAvailableClients( + device: null | undefined | BaseDevice, + clients: Client[], +): Client[] { + if (!device) { + return []; + } + return clients + .filter( + (client: Client) => + (device && + device.supportsOS(client.query.os) && + client.query.device_id === device.serial) || + // Old android sdk versions don't know their device_id + // Display their plugins under all selected devices until they die out + client.query.device_id === 'unknown', + ) + .sort((a, b) => (a.query.app || '').localeCompare(b.query.app)); +} + +function getBestAvailableClient( + device: BaseDevice | null | undefined, + clients: Client[], + preferredClient: string | null, +): Client | undefined { + const availableClients = getAvailableClients(device, clients); + if (availableClients.length === 0) { + return undefined; + } + return ( + getClientById(availableClients, preferredClient) || + availableClients[0] || + null + ); +} + +export function getClientById( + clients: Client[], + clientId: string | null | undefined, +): Client | undefined { + return clients.find(client => client.id === clientId); +} + +function canBeDefaultDevice(device: BaseDevice) { + return !DEFAULT_DEVICE_BLACKLIST.some( + blacklistedDevice => device instanceof blacklistedDevice, + ); +} + +/** + * This function, given the current state, tries to build to build the best + * selection possible, preselection device if there is non, plugins based on preferences, etc + * @param state + */ +function updateSelection(state: Readonly): State { + const updates: Partial = { + staticView: null, + }; + // Find the selected device if it still exists + let device: BaseDevice | null = + state.selectedDevice && state.devices.includes(state.selectedDevice) + ? state.selectedDevice + : null; + if (!device) { + device = + state.devices.find( + device => device.title === state.userPreferredDevice, + ) || + state.devices.find(device => canBeDefaultDevice(device)) || + null; + } + updates.selectedDevice = device; + if (!device) { + updates.staticView = WelcomeScreen; + } + + // Select client based on device + const client = getBestAvailableClient( + device, + state.clients, + state.selectedApp || state.userPreferredApp, + ); + updates.selectedApp = client ? client.id : null; + + const availablePlugins: string[] = [ + ...(device?.devicePlugins || []), + ...(client?.plugins || []), + ]; + + if ( + // Try the preferred plugin first + state.userPreferredPlugin && + availablePlugins.includes(state.userPreferredPlugin) + ) { + updates.selectedPlugin = state.userPreferredPlugin; + } else if ( + !state.selectedPlugin || + !availablePlugins.includes(state.selectedPlugin) + ) { + // currently selected plugin is not available in this state, + // fall back to the default + updates.selectedPlugin = DEFAULT_PLUGIN; + } + + const res = {...state, ...updates}; + console.log(res.selectedDevice, res.selectedApp, res.selectedPlugin); + return res; +} diff --git a/src/reducers/index.tsx b/src/reducers/index.tsx index 66f3093d9..a30f1e14f 100644 --- a/src/reducers/index.tsx +++ b/src/reducers/index.tsx @@ -97,7 +97,6 @@ export default combineReducers({ 'userPreferredPlugin', 'userPreferredApp', 'userStarredPlugins', - 'selectedClient', ], }, connections, diff --git a/src/utils/exportData.tsx b/src/utils/exportData.tsx index 3a26c9c37..a6f1b0b8d 100644 --- a/src/utils/exportData.tsx +++ b/src/utils/exportData.tsx @@ -468,7 +468,7 @@ export async function getStoreExport( const state = store.getState(); const {clients} = state.connections; const client = clients.find( - client => client.query.app === state.connections.selectedClient, + client => client.query.app === state.connections.selectedApp, ); const {pluginStates} = state; const {plugins} = state; @@ -616,6 +616,7 @@ export function importDataToStore(source: string, data: string, store: Store) { }); return; } + archivedDevice.loadDevicePlugins(store.getState().plugins.devicePlugins); store.dispatch({ type: 'REGISTER_DEVICE', payload: archivedDevice, diff --git a/src/utils/icons.js b/src/utils/icons.js index 855cb881f..a70a388ff 100644 --- a/src/utils/icons.js +++ b/src/utils/icons.js @@ -32,6 +32,7 @@ const ICONS = { 'magic-wand': [20], 'magnifying-glass': [16, 20], 'minus-circle': [12], + 'mobile-engagement': [16], 'question-circle-outline': [16], 'star-outline': [12, 16, 24], 'triangle-down': [12], @@ -47,7 +48,7 @@ const ICONS = { desktop: [12], directions: [12], internet: [12], - mobile: [12], + mobile: [12, 32], posts: [20], profile: [12], rocket: [20],