From d687e27c9c52af59a2501e24d2b868cf10689a6a Mon Sep 17 00:00:00 2001 From: koonpeng Date: Tue, 1 Sep 2020 17:05:48 +0800 Subject: [PATCH 01/16] only render each door items when its door state changes --- package-lock.json | 76 ++++++++---- package.json | 2 +- src/app-contexts.ts | 3 + src/components/app.tsx | 66 ++++------- src/components/door-item.tsx | 8 +- src/components/doors-panel.tsx | 111 +++++++++++++----- .../schedule-visualizer/doors-overlay.tsx | 7 +- src/components/tests/doors-panel.test.tsx | 2 +- src/door-state-manager.ts | 15 ++- src/mock/fake-transport.ts | 39 ++++-- src/stories/baseComponents/door-panel.tsx | 25 ---- src/stories/door.stories.tsx | 14 ++- 12 files changed, 222 insertions(+), 146 deletions(-) delete mode 100644 src/stories/baseComponents/door-panel.tsx diff --git a/package-lock.json b/package-lock.json index b200d4640..a594f583d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5215,20 +5215,39 @@ } }, "airbnb-prop-types": { - "version": "2.15.0", - "resolved": "https://registry.npmjs.org/airbnb-prop-types/-/airbnb-prop-types-2.15.0.tgz", - "integrity": "sha512-jUh2/hfKsRjNFC4XONQrxo/n/3GG4Tn6Hl0WlFQN5PY9OMC9loSCoAYKnZsWaP8wEfd5xcrPloK0Zg6iS1xwVA==", + "version": "2.16.0", + "resolved": "https://registry.npmjs.org/airbnb-prop-types/-/airbnb-prop-types-2.16.0.tgz", + "integrity": "sha512-7WHOFolP/6cS96PhKNrslCLMYAI8yB1Pp6u6XmxozQOiZbsI5ycglZr5cHhBFfuRcQQjzCMith5ZPZdYiJCxUg==", "requires": { - "array.prototype.find": "^2.1.0", - "function.prototype.name": "^1.1.1", - "has": "^1.0.3", - "is-regex": "^1.0.4", - "object-is": "^1.0.1", + "array.prototype.find": "^2.1.1", + "function.prototype.name": "^1.1.2", + "is-regex": "^1.1.0", + "object-is": "^1.1.2", "object.assign": "^4.1.0", - "object.entries": "^1.1.0", + "object.entries": "^1.1.2", "prop-types": "^15.7.2", "prop-types-exact": "^1.2.0", - "react-is": "^16.9.0" + "react-is": "^16.13.1" + }, + "dependencies": { + "is-regex": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/is-regex/-/is-regex-1.1.1.tgz", + "integrity": "sha512-1+QkEcxiLlB7VEyFtyBg94e08OAsvq7FUBgApTq/w2ymCLyKJgDPsybBENVtA7XCQEgEXxKPonG+mvYRxh/LIg==", + "requires": { + "has-symbols": "^1.0.1" + } + }, + "object.entries": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/object.entries/-/object.entries-1.1.2.tgz", + "integrity": "sha512-BQdB9qKmb/HyNdMNWVr7O3+z5MUIx3aiegEIJqjMBbBf0YT9RRxTJSim4mzFqtyr7PDAHigq0N9dO0m0tRakQA==", + "requires": { + "define-properties": "^1.1.3", + "es-abstract": "^1.17.5", + "has": "^1.0.3" + } + } } }, "ajv": { @@ -9539,27 +9558,38 @@ } }, "enzyme-adapter-react-16": { - "version": "1.15.2", - "resolved": "https://registry.npmjs.org/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.15.2.tgz", - "integrity": "sha512-SkvDrb8xU3lSxID8Qic9rB8pvevDbLybxPK6D/vW7PrT0s2Cl/zJYuXvsd1EBTz0q4o3iqG3FJhpYz3nUNpM2Q==", + "version": "1.15.4", + "resolved": "https://registry.npmjs.org/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.15.4.tgz", + "integrity": "sha512-wPzxs+JaGDK2TPYzl5a9YWGce6i2SQ3Cg51ScLeyj2WotUZ8Obcq1ke/U1Y2VGpYlb9rrX2yCjzSMgtKCeAt5w==", "requires": { - "enzyme-adapter-utils": "^1.13.0", - "enzyme-shallow-equal": "^1.0.1", + "enzyme-adapter-utils": "^1.13.1", + "enzyme-shallow-equal": "^1.0.4", "has": "^1.0.3", "object.assign": "^4.1.0", "object.values": "^1.1.1", "prop-types": "^15.7.2", - "react-is": "^16.12.0", + "react-is": "^16.13.1", "react-test-renderer": "^16.0.0-0", "semver": "^5.7.0" + }, + "dependencies": { + "enzyme-shallow-equal": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/enzyme-shallow-equal/-/enzyme-shallow-equal-1.0.4.tgz", + "integrity": "sha512-MttIwB8kKxypwHvRynuC3ahyNc+cFbR8mjVIltnmzQ0uKGqmsfO4bfBuLxb0beLNPhjblUEYvEbsg+VSygvF1Q==", + "requires": { + "has": "^1.0.3", + "object-is": "^1.1.2" + } + } } }, "enzyme-adapter-utils": { - "version": "1.13.0", - "resolved": "https://registry.npmjs.org/enzyme-adapter-utils/-/enzyme-adapter-utils-1.13.0.tgz", - "integrity": "sha512-YuEtfQp76Lj5TG1NvtP2eGJnFKogk/zT70fyYHXK2j3v6CtuHqc8YmgH/vaiBfL8K1SgVVbQXtTcgQZFwzTVyQ==", + "version": "1.13.1", + "resolved": "https://registry.npmjs.org/enzyme-adapter-utils/-/enzyme-adapter-utils-1.13.1.tgz", + "integrity": "sha512-5A9MXXgmh/Tkvee3bL/9RCAAgleHqFnsurTYCbymecO4ohvtNO5zqIhHxV370t7nJAwaCfkgtffarKpC0GPt0g==", "requires": { - "airbnb-prop-types": "^2.15.0", + "airbnb-prop-types": "^2.16.0", "function.prototype.name": "^1.1.2", "object.assign": "^4.1.0", "object.fromentries": "^2.0.2", @@ -16002,9 +16032,9 @@ "integrity": "sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc=" }, "nearley": { - "version": "2.19.2", - "resolved": "https://registry.npmjs.org/nearley/-/nearley-2.19.2.tgz", - "integrity": "sha512-h6lygT0BWAGErDvoE2LfI+tDeY2+UUrqG5dcBPdCmjnjud9z1wE0P7ljb85iNbE93YA+xJLpoSYGMuUqhnSSSA==", + "version": "2.19.6", + "resolved": "https://registry.npmjs.org/nearley/-/nearley-2.19.6.tgz", + "integrity": "sha512-OV3Lx+o5iIGWVY38zs+7aiSnBqaHTFAOQiz83VHJje/wOOaSgzE3H0S/xfISxJhFSoPcX611OEDV9sCT8F283g==", "requires": { "commander": "^2.19.0", "moo": "^0.5.0", diff --git a/package.json b/package.json index 12aa9e621..4cd160f09 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "clsx": "^1.1.0", "debug": "^4.1.1", "enzyme": "^3.11.0", - "enzyme-adapter-react-16": "^1.15.2", + "enzyme-adapter-react-16": "^1.15.4", "eventemitter3": "^4.0.0", "leaflet": "^1.6.0", "react": "^16.12.0", diff --git a/src/app-contexts.ts b/src/app-contexts.ts index 19de11214..c642a4f14 100644 --- a/src/app-contexts.ts +++ b/src/app-contexts.ts @@ -1,5 +1,8 @@ +import * as RomiCore from '@osrf/romi-js-core-interfaces'; import { createContext } from 'react'; import { ResourceConfigurationsType } from './resource-manager'; /* Declares the ResourcesContext which contains the resources used on the app*/ export const ResourcesContext = createContext({}); + +export const DoorStateContext = createContext>({}); diff --git a/src/components/app.tsx b/src/components/app.tsx index a64d0c5c0..6e6375d49 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -1,14 +1,16 @@ import { AppBar, Fade, IconButton, makeStyles, Toolbar, Typography } from '@material-ui/core/'; import { Dashboard as DashboardIcon, Settings as SettingsIcon } from '@material-ui/icons'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; -import debug from 'debug'; +import Debug from 'debug'; import React from 'react'; import 'typeface-roboto'; import { AppConfig } from '../app-config'; +import { DoorStateContext } from '../app-contexts'; import DispenserStateManager from '../dispenser-state-manager'; import DoorStateManager from '../door-state-manager'; import FleetManager from '../fleet-manager'; import LiftStateManager from '../lift-state-manager'; +import { ResourceConfigurationsType } from '../resource-manager'; import { RobotTrajectoryManager } from '../robot-trajectory-manager'; import { loadSettings, saveSettings, Settings, SettingsContext } from '../settings'; import './app.css'; @@ -18,17 +20,16 @@ import DoorsPanel from './doors-panel'; import LiftsPanel from './lift-item/lifts-panel'; import LoadingScreen, { LoadingScreenProps } from './loading-screen'; import MainMenu from './main-menu'; +import NotificationBar, { NotificationBarContext, NotificationBarProps } from './notification-bar'; import OmniPanel from './omni-panel'; import OmniPanelView from './omni-panel-view'; import RobotsPanel from './robots-panel'; import ScheduleVisualizer from './schedule-visualizer'; +import { LiftStateContext } from './schedule-visualizer/lift-overlay'; import SettingsDrawer from './settings-drawer'; import { SpotlightValue } from './spotlight-value'; -import { DoorStateContext } from './schedule-visualizer/doors-overlay'; -import { LiftStateContext } from './schedule-visualizer/lift-overlay'; -import NotificationBar, { NotificationBarProps, NotificationBarContext } from './notification-bar'; -import { ResourceConfigurationsType } from '../resource-manager'; +const debug = Debug('log'); const borderRadius = 20; const useStyles = makeStyles(theme => ({ @@ -221,26 +222,14 @@ export default function App(props: AppProps): JSX.Element { })(); }, [appResources]); - React.useEffect(() => { - if (currentView === OmniPanelViewIndex.Doors) { - const listener = () => setDoorStates(doorStateManager.doorStates()); - doorStateManager.on('updated', listener); - debug.log('started tracking door states'); - return () => { - doorStateManager.off('updated', listener); - debug.log('stopped tracking door states'); - }; - } - }, [currentView, doorStateManager]); - React.useEffect(() => { if (currentView === OmniPanelViewIndex.Lifts) { const listener = () => setLiftStates(liftStateManager.liftStates()); liftStateManager.on('updated', listener); - debug.log('started tracking lift states'); + debug('started tracking lift states'); return () => { liftStateManager.off('updated', listener); - debug.log('stopped tracking lift states'); + debug('stopped tracking lift states'); }; } }, [currentView, liftStateManager]); @@ -249,10 +238,10 @@ export default function App(props: AppProps): JSX.Element { if (currentView === OmniPanelViewIndex.Dispensers) { const listener = () => setDispenserStates(dispenserStateManager.dispenserStates()); dispenserStateManager.on('updated', listener); - debug.log('started tracking dispenser states'); + debug('started tracking dispenser states'); return () => { dispenserStateManager.off('updated', listener); - debug.log('stopped tracking dispenser states'); + debug('stopped tracking dispenser states'); }; } }, [currentView, dispenserStateManager]); @@ -302,7 +291,6 @@ export default function App(props: AppProps): JSX.Element { } function handleMainMenuDoorsClick(): void { - setDoorStates(doorStateManager.doorStates()); setCurrentView(OmniPanelViewIndex.Doors); } @@ -324,8 +312,8 @@ export default function App(props: AppProps): JSX.Element { } return ( - - + + {loading && }
@@ -343,19 +331,17 @@ export default function App(props: AppProps): JSX.Element { {buildingMap && ( - - - - - + + + )} @@ -422,7 +408,7 @@ export default function App(props: AppProps): JSX.Element { type={notificationBarMessage?.type} /> - - + + ); } diff --git a/src/components/door-item.tsx b/src/components/door-item.tsx index c4dd75eb0..29aedd7b0 100644 --- a/src/components/door-item.tsx +++ b/src/components/door-item.tsx @@ -12,10 +12,12 @@ import { } from '@material-ui/core'; import { ExpandMore as ExpandMoreIcon } from '@material-ui/icons'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; - -import OmniPanelStatusLabels from './omni-panel-status-labels'; import { colorPalette } from '../util/css-utils'; +import OmniPanelStatusLabels from './omni-panel-status-labels'; + +const debug = Debug('DoorItem'); export interface DoorItemProps extends Omit { door: Readonly; @@ -30,6 +32,8 @@ export const DoorItem = React.forwardRef(function( props: DoorItemProps, ref: React.Ref, ): React.ReactElement { + debug('render'); + const { door, doorState, enableControls, onOpenClick, onCloseClick, ...otherProps } = props; const classes = useStyles(); const theme = useTheme(); diff --git a/src/components/doors-panel.tsx b/src/components/doors-panel.tsx index 1b31f1cf7..f6f4135c7 100644 --- a/src/components/doors-panel.tsx +++ b/src/components/doors-panel.tsx @@ -1,8 +1,12 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; -import DoorItem from './door-item'; +import DoorItem_, { DoorItemProps } from './door-item'; import { SpotlightValue } from './spotlight-value'; +const debug = Debug('DoorsPanel'); +const DoorItem = React.memo(DoorItem_); + export interface DoorsPanelProps { doors: readonly RomiCore.Door[]; doorStates: Readonly>; @@ -11,29 +15,83 @@ export interface DoorsPanelProps { onDoorClick?(door: RomiCore.Door): void; } -export default function DoorsPanel(props: DoorsPanelProps): JSX.Element { - const { transport, spotlight, onDoorClick } = props; - const doorRefs = React.useRef>({}); +function requestDoor( + publisher: RomiCore.Publisher, + requester_id: string, + door: RomiCore.Door, + mode: number, +): void { + publisher.publish({ + door_name: door.name, + requested_mode: { value: mode }, + requester_id: requester_id, + request_time: RomiCore.toRosTime(new Date()), + }); +} + +export const DoorsPanel = React.memo((props: DoorsPanelProps) => { + debug('render'); + + const { doors, doorStates, transport, spotlight, onDoorClick } = props; + const doorRefs = React.useMemo(() => { + const refs: Record> = {}; + doors.map(door => (refs[door.name] = React.createRef())); + return refs; + }, [doors]); const [expanded, setExpanded] = React.useState>({}); const doorRequestPub = React.useMemo( () => (transport ? transport.createPublisher(RomiCore.adapterDoorRequests) : null), [transport], ); - function requestDoor(door: RomiCore.Door, mode: number): void { - doorRequestPub?.publish({ - door_name: door.name, - requested_mode: { value: mode }, - requester_id: transport!.name, - request_time: RomiCore.toRosTime(new Date()), - }); - } + const onChange = React.useMemo(() => { + return doors.reduce>((prev, door) => { + prev[door.name] = (_, newExpanded) => { + setExpanded(prev => ({ + ...prev, + [door.name]: newExpanded, + })); + }; + return prev; + }, {}); + }, [doors]); + + const onClick = React.useMemo(() => { + return doors.reduce>((prev, door) => { + prev[door.name] = () => { + onDoorClick && onDoorClick(door); + }; + return prev; + }, {}); + }, [doors, onDoorClick]); + + const onOpenClick = React.useMemo(() => { + return doors.reduce>((prev, door) => { + prev[door.name] = () => { + doorRequestPub && + transport && + requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_OPEN); + }; + return prev; + }, {}); + }, [doors, doorRequestPub, transport]); + + const onCloseClick = React.useMemo(() => { + return doors.reduce>((prev, door) => { + prev[door.name] = () => { + doorRequestPub && + transport && + requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_CLOSED); + }; + return prev; + }, {}); + }, [doors, doorRequestPub, transport]); React.useEffect(() => { if (!spotlight) { return; } - const ref = doorRefs.current[spotlight.value]; + const ref = doorRefs[spotlight.value]; if (!ref) { return; } @@ -41,31 +99,28 @@ export default function DoorsPanel(props: DoorsPanelProps): JSX.Element { ...prev, [spotlight.value]: true, })); - ref.scrollIntoView({ behavior: 'smooth' }); - }, [spotlight]); + ref.current?.scrollIntoView({ behavior: 'smooth' }); + }, [spotlight, doorRefs]); - const listItems = props.doors.map(door => { - const doorState = props.doorStates[door.name]; + const listItems = doors.map(door => { + const doorState = doorStates[door.name]; return ( (doorRefs.current[door.name] = ref)} + ref={doorRefs[door.name]} door={door} doorState={doorState} enableControls={Boolean(transport)} - onOpenClick={() => requestDoor(door, RomiCore.DoorMode.MODE_OPEN)} - onCloseClick={() => requestDoor(door, RomiCore.DoorMode.MODE_CLOSED)} - onClick={() => onDoorClick && onDoorClick(door)} + onOpenClick={onOpenClick[door.name]} + onCloseClick={onCloseClick[door.name]} + onClick={onClick[door.name]} expanded={Boolean(expanded[door.name])} - onChange={(_, newExpanded) => - setExpanded(prev => ({ - ...prev, - [door.name]: newExpanded, - })) - } + onChange={onChange[door.name]} /> ); }); return {listItems}; -} +}); + +export default DoorsPanel; diff --git a/src/components/schedule-visualizer/doors-overlay.tsx b/src/components/schedule-visualizer/doors-overlay.tsx index 302eb0a44..c3e8a79b9 100644 --- a/src/components/schedule-visualizer/doors-overlay.tsx +++ b/src/components/schedule-visualizer/doors-overlay.tsx @@ -4,11 +4,12 @@ */ import * as RomiCore from '@osrf/romi-js-core-interfaces'; -import React, { createContext, useContext } from 'react'; +import React, { useContext } from 'react'; +import { DoorStateContext } from '../../app-contexts'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; +import Door from './door/door'; import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; -import Door from './door/door'; export interface DoorsOverlayProps extends SVGOverlayProps { doors: readonly RomiCore.Door[]; onDoorClick?(door: RomiCore.Door): void; @@ -41,5 +42,3 @@ export default function DoorsOverlay(props: DoorsOverlayProps): React.ReactEleme ); } - -export const DoorStateContext = createContext>>({}); diff --git a/src/components/tests/doors-panel.test.tsx b/src/components/tests/doors-panel.test.tsx index 1bdc8dec7..c27c2e493 100644 --- a/src/components/tests/doors-panel.test.tsx +++ b/src/components/tests/doors-panel.test.tsx @@ -23,7 +23,7 @@ beforeEach(async () => { it('renders doors', () => { const root = mount(); - const doorElements = root.find(DoorItem); + const doorElements = root.find('[data-component="DoorItem"]').hostNodes(); expect(doorElements.length).toBe(doors.length); root.unmount(); }); diff --git a/src/door-state-manager.ts b/src/door-state-manager.ts index b97da8342..72657b841 100644 --- a/src/door-state-manager.ts +++ b/src/door-state-manager.ts @@ -11,11 +11,18 @@ export default class DoorStateManager extends EventEmitter { } startSubscription(transport: RomiCore.Transport) { - transport.subscribe(RomiCore.doorStates, doorState => { - this._doorStates[doorState.door_name] = doorState; - this.emit('updated'); - }); + this._subscriptions.push( + transport.subscribe(RomiCore.doorStates, doorState => { + this._doorStates[doorState.door_name] = doorState; + this.emit('updated'); + }), + ); + } + + stopAllSubscriptions(): void { + this._subscriptions.forEach(sub => sub.unsubscribe()); } private _doorStates: Record = {}; + private _subscriptions: RomiCore.Subscription[] = []; } diff --git a/src/mock/fake-transport.ts b/src/mock/fake-transport.ts index d6a016b63..d72b0a9a8 100644 --- a/src/mock/fake-transport.ts +++ b/src/mock/fake-transport.ts @@ -1,20 +1,29 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; -import debug from 'debug'; +import Debug from 'debug'; import buildingMap from './data/building-map'; import fakeDispenserStates from './data/dispenser-states'; import fakeDoorStates from './data/door-states'; import fakeFleets from './data/fleets'; import fakeLiftStates from './data/lift-states'; +const debug = Debug('FakeTransport'); + +export type DoorStateFactory = () => Record; + export class FakeTransport extends RomiCore.TransportEvents implements RomiCore.Transport { name: string = 'fake'; + constructor(doorStateFactory?: DoorStateFactory) { + super(); + this._doorStateFactory = doorStateFactory || fakeDoorStates; + } + createPublisher( topic: RomiCore.RomiTopic, options?: RomiCore.Options | undefined, ): RomiCore.Publisher { return { - publish: debug.log, + publish: debug, }; } @@ -23,12 +32,14 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. cb: RomiCore.SubscriptionCb, options?: RomiCore.Options | undefined, ): RomiCore.Subscription { - debug.log('subscribe:', topic); + debug('subscribe %s', topic.topic); + let timer: number; switch (topic) { case RomiCore.doorStates: { - const doorStates = fakeDoorStates(); - setInterval(() => { + const doorStates = this._doorStateFactory(); + timer = window.setInterval(() => { for (const state of Object.values(doorStates)) { + debug('publishing door state'); cb(state as Message); } }, 1000); @@ -36,7 +47,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } case RomiCore.liftStates: { const liftStates = fakeLiftStates(); - setInterval(() => { + timer = window.setInterval(() => { for (const state of Object.values(liftStates)) { cb(state as Message); } @@ -45,7 +56,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } case RomiCore.fleetStates: { const fleets = fakeFleets(); - setInterval(() => { + timer = window.setInterval(() => { for (const fleet of fleets) { cb(fleet as Message); } @@ -54,7 +65,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } case RomiCore.dispenserStates: { const dispenserStates = fakeDispenserStates(); - setInterval(() => { + timer = window.setInterval(() => { for (const state of Object.values(dispenserStates)) { cb(state as Message); } @@ -63,10 +74,9 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } } - // TODO remove the skip validation when the type problem of available_floors is solved if (topic !== RomiCore.liftStates && topic.topic === 'lift_states') { const liftStates = fakeLiftStates(); - setInterval(() => { + timer = window.setInterval(() => { for (const state of Object.values(liftStates)) { cb(state as Message); } @@ -74,7 +84,10 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } return { - unsubscribe: () => { }, + unsubscribe: () => { + clearInterval(timer); + debug('unsubscribed %s', topic.topic); + }, }; } @@ -94,7 +107,9 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. throw new Error('not implemented'); } - destroy(): void { } + destroy(): void {} + + private _doorStateFactory: DoorStateFactory; } export default FakeTransport; diff --git a/src/stories/baseComponents/door-panel.tsx b/src/stories/baseComponents/door-panel.tsx deleted file mode 100644 index ebd541505..000000000 --- a/src/stories/baseComponents/door-panel.tsx +++ /dev/null @@ -1,25 +0,0 @@ -import React from 'react'; - -import DoorItem from '../../components/door-item'; -import { DoorsPanelProps } from '../../components/doors-panel'; - -export default function DoorButton(props: DoorsPanelProps) { - const { doors, doorStates } = props; - const doorRefs = React.useRef>({}); - - return ( - - {doors.map(door => { - const doorState = doorStates[door.name]; - return ( - (doorRefs.current[door.name] = ref)} - door={door} - doorState={doorState} - /> - ); - })} - - ); -} diff --git a/src/stories/door.stories.tsx b/src/stories/door.stories.tsx index caf6e26e2..d313626c9 100644 --- a/src/stories/door.stories.tsx +++ b/src/stories/door.stories.tsx @@ -1,15 +1,17 @@ -import React from 'react'; import { Divider, Typography } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; - +import React from 'react'; +import DoorsPanel from '../components/doors-panel'; +import DoorStateManager from '../door-state-manager'; +import FakeTransport from '../mock/fake-transport'; import DoorComponent from './baseComponents/door-component'; -import DoorButton from './baseComponents/door-panel'; +// import DoorButton from './baseComponents/door-panel'; import { + componentDisplayStyle, + defaultStyles, door, doors, doorStates, - componentDisplayStyle, - defaultStyles, StyleTyping, } from './baseComponents/utils'; @@ -126,6 +128,6 @@ export const doorPanel = () => ( Door State Button color and representation
- + ); From 4f664d5249e5137009fa7ef7ff6e5318b8b027d6 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 11:21:27 +0800 Subject: [PATCH 02/16] helper function and explaination for using callbacks with memo --- src/components/app.tsx | 2 +- src/components/doors-panel.tsx | 80 +++++++++++++++++----------------- src/util/react-helpers.ts | 60 +++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 41 deletions(-) create mode 100644 src/util/react-helpers.ts diff --git a/src/components/app.tsx b/src/components/app.tsx index 6e6375d49..4f423c018 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -121,7 +121,7 @@ export default function App(props: AppProps): JSX.Element { const doorStateManager = React.useMemo(() => new DoorStateManager(), []); const [doorStates, setDoorStates] = React.useState(() => doorStateManager.doorStates()); - const [doors, setDoors] = React.useState([]); + const [doors, setDoors] = React.useState([]); const [doorSpotlight, setDoorSpotlight] = React.useState | undefined>( undefined, diff --git a/src/components/doors-panel.tsx b/src/components/doors-panel.tsx index f6f4135c7..1f9c304bb 100644 --- a/src/components/doors-panel.tsx +++ b/src/components/doors-panel.tsx @@ -1,6 +1,7 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; import Debug from 'debug'; import React from 'react'; +import { makeCallbackArrayCallback } from '../util/react-helpers'; import DoorItem_, { DoorItemProps } from './door-item'; import { SpotlightValue } from './spotlight-value'; @@ -8,7 +9,7 @@ const debug = Debug('DoorsPanel'); const DoorItem = React.memo(DoorItem_); export interface DoorsPanelProps { - doors: readonly RomiCore.Door[]; + doors: RomiCore.Door[]; doorStates: Readonly>; transport?: Readonly; spotlight?: Readonly>; @@ -44,48 +45,47 @@ export const DoorsPanel = React.memo((props: DoorsPanelProps) => { [transport], ); - const onChange = React.useMemo(() => { - return doors.reduce>((prev, door) => { - prev[door.name] = (_, newExpanded) => { + const onChange = React.useMemo( + makeCallbackArrayCallback['onChange'], RomiCore.Door>( + doors, + (door, _, newExpanded) => setExpanded(prev => ({ ...prev, [door.name]: newExpanded, - })); - }; - return prev; - }, {}); - }, [doors]); + })), + ), + [doors], + ); - const onClick = React.useMemo(() => { - return doors.reduce>((prev, door) => { - prev[door.name] = () => { - onDoorClick && onDoorClick(door); - }; - return prev; - }, {}); - }, [doors, onDoorClick]); + const onClick = React.useMemo( + makeCallbackArrayCallback['onClick'], RomiCore.Door>( + doors, + door => onDoorClick && onDoorClick(door), + ), + [doors, onDoorClick], + ); - const onOpenClick = React.useMemo(() => { - return doors.reduce>((prev, door) => { - prev[door.name] = () => { + const onOpenClick = React.useMemo( + makeCallbackArrayCallback['onOpenClick'], RomiCore.Door>( + doors, + door => doorRequestPub && - transport && - requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_OPEN); - }; - return prev; - }, {}); - }, [doors, doorRequestPub, transport]); + transport && + requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_OPEN), + ), + [doors, onDoorClick], + ); - const onCloseClick = React.useMemo(() => { - return doors.reduce>((prev, door) => { - prev[door.name] = () => { + const onCloseClick = React.useMemo( + makeCallbackArrayCallback['onCloseClick'], RomiCore.Door>( + doors, + door => doorRequestPub && - transport && - requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_CLOSED); - }; - return prev; - }, {}); - }, [doors, doorRequestPub, transport]); + transport && + requestDoor(doorRequestPub, transport.name, door, RomiCore.DoorMode.MODE_OPEN), + ), + [doors, onDoorClick], + ); React.useEffect(() => { if (!spotlight) { @@ -102,7 +102,7 @@ export const DoorsPanel = React.memo((props: DoorsPanelProps) => { ref.current?.scrollIntoView({ behavior: 'smooth' }); }, [spotlight, doorRefs]); - const listItems = doors.map(door => { + const listItems = doors.map((door, i) => { const doorState = doorStates[door.name]; return ( { door={door} doorState={doorState} enableControls={Boolean(transport)} - onOpenClick={onOpenClick[door.name]} - onCloseClick={onCloseClick[door.name]} - onClick={onClick[door.name]} + onOpenClick={onOpenClick[i]} + onCloseClick={onCloseClick[i]} + onClick={onClick[i]} expanded={Boolean(expanded[door.name])} - onChange={onChange[door.name]} + onChange={onChange[i]} /> ); }); diff --git a/src/util/react-helpers.ts b/src/util/react-helpers.ts new file mode 100644 index 000000000..86d8ab151 --- /dev/null +++ b/src/util/react-helpers.ts @@ -0,0 +1,60 @@ +/** + * Helper function to create a callback that returns a map of callbacks from a Record. Meant to be + * used as the callback for `React.useMemo` to create callbacks that match equality test at each + * render. + * + * Given this render + * ```tsx + * {items.map((i) => } + * ``` + * This looks fine at first but this causes problems when using `React.memo` because at each render + * new functions are created which doesn't equal to the previous (functions in js are only equal + * to itself, identical functions are never equal). + * + * The naive approach would be to use `React.useCallback` but that hits another problem, you can't + * use hooks in a callback and the `ListItem` is generated in the `map` callback. + * + * ```tsx + * /// doesn't work! + * {items.map((i) => doSomething(i.value))} />} + * ``` + * + * So then how about using `React.useCallback` outside the `map` function? Then you lost your + * reference to `i`. + * + * ```tsx + * // `i` not defined! + * React.useCallback(() => doSomething(i.value)); + * ``` + * + * So then the solution is to create an array of callbacks, one for each item and wrap them in + * `React.useMemo`, this way the same callback is used each render and each callback can receive + * a reference to the `i` object. + * + * ```tsx + * // ok! + * React.useMemo(() => items.map((i) => callback(i, ...onClickArgs))); + * ``` + * + * @example + * ```ts + * const onClick = React.useMemo( + * makeCallbackMapCallback['onClick']>( + * doorsMap, + * (_, door) => onDoorClick && onDoorClick(door), + * ), + * [doorsMap, onDoorClick], + * ); + * ``` + * + * @param arr + * @param callback + */ +export function makeCallbackArrayCallback< + U extends (...args: any[]) => any = (...args: any[]) => any, + T = any, + Args extends any[] = U extends (...args: infer Args) => any ? Args : never, + R = U extends (...args: any[]) => infer R ? R : never +>(arr: T[], callback: (item: T, ...args: Args) => R): () => ((...args: Args) => R)[] { + return () => arr.map(item => (...args: Args) => callback(item, ...args)); +} From 4c5896db2c4ebcf24aaf14d0380b90c1c94149ba Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 11:54:55 +0800 Subject: [PATCH 03/16] optimize lifts panel render --- src/components/app.tsx | 182 ++++++++++------------ src/components/doors-panel.tsx | 11 +- src/components/lift-item/lift-item.tsx | 134 ++++++++-------- src/components/lift-item/lifts-panel.tsx | 124 ++++++++++----- src/lift-state-manager.ts | 29 ++-- src/mock/fake-transport.ts | 1 + src/stories/baseComponents/lift-panel.tsx | 26 ---- src/stories/door.stories.tsx | 4 +- src/stories/lift.stories.tsx | 11 +- 9 files changed, 269 insertions(+), 253 deletions(-) delete mode 100644 src/stories/baseComponents/lift-panel.tsx diff --git a/src/components/app.tsx b/src/components/app.tsx index 4f423c018..462e0ed7c 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -122,15 +122,13 @@ export default function App(props: AppProps): JSX.Element { const doorStateManager = React.useMemo(() => new DoorStateManager(), []); const [doorStates, setDoorStates] = React.useState(() => doorStateManager.doorStates()); const [doors, setDoors] = React.useState([]); - const [doorSpotlight, setDoorSpotlight] = React.useState | undefined>( undefined, ); const liftStateManager = React.useMemo(() => new LiftStateManager(), []); const [liftStates, setLiftStates] = React.useState(() => liftStateManager.liftStates()); - - const [lifts, setLifts] = React.useState([]); + const [lifts, setLifts] = React.useState([]); const [liftSpotlight, setLiftSpotlight] = React.useState | undefined>( undefined, ); @@ -222,18 +220,6 @@ export default function App(props: AppProps): JSX.Element { })(); }, [appResources]); - React.useEffect(() => { - if (currentView === OmniPanelViewIndex.Lifts) { - const listener = () => setLiftStates(liftStateManager.liftStates()); - liftStateManager.on('updated', listener); - debug('started tracking lift states'); - return () => { - liftStateManager.off('updated', listener); - debug('stopped tracking lift states'); - }; - } - }, [currentView, liftStateManager]); - React.useEffect(() => { if (currentView === OmniPanelViewIndex.Dispensers) { const listener = () => setDispenserStates(dispenserStateManager.dispenserStates()); @@ -314,24 +300,24 @@ export default function App(props: AppProps): JSX.Element { return ( - - {loading && } -
- - - - Dashboard - - setShowOmniPanel(!showOmniPanel)}> - - - setShowSettings(true)}> - - - - - {buildingMap && ( - + + + {loading && } +
+ + + + Dashboard + + setShowOmniPanel(!showOmniPanel)}> + + + setShowSettings(true)}> + + + + + {buildingMap && ( - - )} - - + + + + + + + + + + + + + + + + + + + + + + { + setSettings(newSettings); + saveSettings(newSettings); }} - view={currentView} - onBack={handleBack} - onClose={handleClose} - > - - - - - - - - - - - - - - - - - - - - - { - setSettings(newSettings); - saveSettings(newSettings); - }} - onClose={() => setShowSettings(false)} + onClose={() => setShowSettings(false)} + /> +
+ -
- - + + ); diff --git a/src/components/doors-panel.tsx b/src/components/doors-panel.tsx index 1f9c304bb..5b4eb386d 100644 --- a/src/components/doors-panel.tsx +++ b/src/components/doors-panel.tsx @@ -34,17 +34,18 @@ export const DoorsPanel = React.memo((props: DoorsPanelProps) => { debug('render'); const { doors, doorStates, transport, spotlight, onDoorClick } = props; - const doorRefs = React.useMemo(() => { - const refs: Record> = {}; - doors.map(door => (refs[door.name] = React.createRef())); - return refs; - }, [doors]); const [expanded, setExpanded] = React.useState>({}); const doorRequestPub = React.useMemo( () => (transport ? transport.createPublisher(RomiCore.adapterDoorRequests) : null), [transport], ); + const doorRefs = React.useMemo(() => { + const refs: Record> = {}; + doors.map(door => (refs[door.name] = React.createRef())); + return refs; + }, [doors]); + const onChange = React.useMemo( makeCallbackArrayCallback['onChange'], RomiCore.Door>( doors, diff --git a/src/components/lift-item/lift-item.tsx b/src/components/lift-item/lift-item.tsx index 9dc3195a5..75aca4f9e 100644 --- a/src/components/lift-item/lift-item.tsx +++ b/src/components/lift-item/lift-item.tsx @@ -14,6 +14,9 @@ import LiftRequestForm from './lift-item-form'; import { LiftRequestManager } from '../../lift-state-manager'; import OmniPanelStatusLabels from '../omni-panel-status-labels'; import { colorPalette } from '../../util/css-utils'; +import Debug from 'debug'; + +const debug = Debug('LiftItem'); export interface LiftItemProps extends Omit { id?: string; @@ -28,77 +31,78 @@ export interface LiftItemProps extends Omit { ): void; } -export const LiftItem = React.forwardRef(function( - props: LiftItemProps, - ref: React.Ref, -): React.ReactElement { - const { id, lift, liftState, enableRequest, onRequest, ...otherProps } = props; - const [tabValue, setTabValue] = React.useState(0); - const classes = useStyles(); - - function liftFloorLabel(liftState?: RomiCore.LiftState): string { - if (!liftState) { - return classes.liftFloorLabelUnknown; +export const LiftItem = React.memo( + React.forwardRef(function(props: LiftItemProps, ref: React.Ref): React.ReactElement { + debug('render'); + + const { id, lift, liftState, enableRequest, onRequest, ...otherProps } = props; + const [tabValue, setTabValue] = React.useState(0); + const classes = useStyles(); + + function liftFloorLabel(liftState?: RomiCore.LiftState): string { + if (!liftState) { + return classes.liftFloorLabelUnknown; + } + switch (liftState.motion_state) { + case RomiCore.LiftState.MOTION_UP: + case RomiCore.LiftState.MOTION_DOWN: + return classes.liftFloorLabelMoving; + default: + return classes.liftFloorLabelStopped; + } } - switch (liftState.motion_state) { - case RomiCore.LiftState.MOTION_UP: - case RomiCore.LiftState.MOTION_DOWN: - return classes.liftFloorLabelMoving; - default: - return classes.liftFloorLabelStopped; + + function handleRequest(doorState: number, requestType: number, destination: string): void { + !!onRequest && onRequest(lift, doorState, requestType, destination); } - } - function handleRequest(doorState: number, requestType: number, destination: string): void { - !!onRequest && onRequest(lift, doorState, requestType, destination); - } + const handleChange = (event: React.ChangeEvent<{}>, newValue: number) => { + setTabValue(newValue); + }; - const handleChange = (event: React.ChangeEvent<{}>, newValue: number) => { - setTabValue(newValue); - }; + const doorStates = React.useMemo(() => LiftRequestManager.getDoorModes(), []); + const requestTypes = React.useMemo(() => LiftRequestManager.getLiftRequestModes(), []); - const doorStates = React.useMemo(() => LiftRequestManager.getDoorModes(), []); - const requestTypes = React.useMemo(() => LiftRequestManager.getLiftRequestModes(), []); - - return ( - - } - > - - - - + } > - - - - - - - - {lift.levels && ( - - )} - - - - ); -}); + + + + + + + + + + + + {lift.levels && ( + + )} + + + + ); + }), +); const useStyles = makeStyles(theme => { const liftFloorLabelBase: CSSProperties = { diff --git a/src/components/lift-item/lifts-panel.tsx b/src/components/lift-item/lifts-panel.tsx index 1844a19ac..653efc39d 100644 --- a/src/components/lift-item/lifts-panel.tsx +++ b/src/components/lift-item/lifts-panel.tsx @@ -1,10 +1,15 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import { LiftRequest } from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; -import { LiftItem } from './lift-item'; +import { makeCallbackArrayCallback } from '../../util/react-helpers'; import { SpotlightValue } from '../spotlight-value'; +import { LiftItem, LiftItemProps } from './lift-item'; + +const debug = Debug('LiftsPanel'); export interface LiftsPanelProps { - lifts: readonly RomiCore.Lift[]; + lifts: RomiCore.Lift[]; liftStates: Readonly>; transport?: Readonly; spotlight?: Readonly>; @@ -12,37 +17,45 @@ export interface LiftsPanelProps { onLiftClick?(lift: RomiCore.Lift): void; } -export default function LiftsPanel(props: LiftsPanelProps): JSX.Element { - const { transport, spotlight, onLiftRequest, onLiftClick } = props; +function handleLiftRequest( + liftRequestPub: RomiCore.Publisher, + sessionId: string, + lift: RomiCore.Lift, + doorState: number, + requestType: number, + destination: string, +): void { + liftRequestPub.publish({ + lift_name: lift.name, + door_state: doorState, + request_type: requestType, + request_time: RomiCore.toRosTime(new Date()), + destination_floor: destination, + session_id: sessionId, + }); +} + +export const LiftsPanel = React.memo((props: LiftsPanelProps) => { + debug('render'); + + const { lifts, liftStates, transport, spotlight, onLiftRequest, onLiftClick } = props; const liftRequestPub = React.useMemo( () => (transport ? transport.createPublisher(RomiCore.adapterLiftRequests) : null), [transport], ); - const liftRefs = React.useRef>({}); const [expanded, setExpanded] = React.useState>>({}); - function handleLiftRequest( - lift: RomiCore.Lift, - doorState: number, - requestType: number, - destination: string, - ): void { - liftRequestPub?.publish({ - lift_name: lift.name, - door_state: doorState, - request_type: requestType, - request_time: RomiCore.toRosTime(new Date()), - destination_floor: destination, - session_id: transport!.name, - }); - onLiftRequest && onLiftRequest(lift, destination); - } + const liftRefs = React.useMemo(() => { + const refs: Record> = {}; + lifts.map(lift => (refs[lift.name] = React.createRef())); + return refs; + }, [lifts]); React.useEffect(() => { if (!spotlight) { return; } - const ref = liftRefs.current[spotlight.value]; + const ref = liftRefs[spotlight.value]; if (!ref) { return; } @@ -50,33 +63,70 @@ export default function LiftsPanel(props: LiftsPanelProps): JSX.Element { ...prev, [spotlight.value]: true, })); - ref.scrollIntoView({ behavior: 'smooth' }); - }, [spotlight]); + ref.current?.scrollIntoView({ behavior: 'smooth' }); + }, [spotlight, liftRefs]); + + const onChange = React.useMemo( + makeCallbackArrayCallback['onChange'], RomiCore.Lift>( + lifts, + (lift, _, newExpanded) => + setExpanded(prev => ({ + ...prev, + [lift.name]: newExpanded, + })), + ), + [lifts], + ); + + const onClick = React.useMemo( + makeCallbackArrayCallback['onClick'], RomiCore.Lift>( + lifts, + lift => onLiftClick && onLiftClick(lift), + ), + [lifts, onLiftClick], + ); + + const onRequest = React.useMemo( + makeCallbackArrayCallback['onRequest'], RomiCore.Lift>( + lifts, + (lift, _, doorState, requestType, destination) => { + liftRequestPub && + transport && + handleLiftRequest( + liftRequestPub, + transport.name, + lift, + doorState, + requestType, + destination, + ); + onLiftRequest && onLiftRequest(lift, destination); + }, + ), + [], + ); return ( - {props.lifts.map(lift => { - const liftState = props.liftStates[lift.name]; + {lifts.map((lift, i) => { + const liftState = liftStates[lift.name]; return ( (liftRefs.current[lift.name] = ref)} + ref={liftRefs[lift.name]} liftState={liftState} enableRequest={Boolean(transport)} - onRequest={handleLiftRequest} - onClick={() => onLiftClick && onLiftClick(lift)} + onRequest={onRequest[i]} + onClick={onClick[i]} expanded={Boolean(expanded[lift.name])} - onChange={(_, newExpanded) => - setExpanded(prev => ({ - ...prev, - [lift.name]: newExpanded, - })) - } + onChange={onChange[i]} /> ); })} ); -} +}); + +export default LiftsPanel; diff --git a/src/lift-state-manager.ts b/src/lift-state-manager.ts index 61be96e84..93da9d90e 100644 --- a/src/lift-state-manager.ts +++ b/src/lift-state-manager.ts @@ -11,11 +11,16 @@ export default class LiftStateManager extends EventEmitter { } startSubscription(transport: RomiCore.Transport) { - // TODO remove the skip validation when the type problem of available_floors is solved - transport.subscribe(RomiCore.skipValidation(RomiCore.liftStates), liftState => { - this._liftStates[liftState.lift_name] = liftState; - this.emit('updated'); - }); + this._subscriptions.push( + transport.subscribe(RomiCore.liftStates, liftState => { + this._liftStates[liftState.lift_name] = liftState; + this.emit('updated'); + }), + ); + } + + stopAllSubscriptions(): void { + this._subscriptions.forEach(sub => sub.unsubscribe()); } static liftModeToString(liftMode: number): string { @@ -62,6 +67,7 @@ export default class LiftStateManager extends EventEmitter { } private _liftStates: Record = {}; + private _subscriptions: RomiCore.Subscription[] = []; } export interface requestManagerModes { @@ -74,27 +80,24 @@ export class LiftRequestManager { RomiCore.LiftRequest.REQUEST_HUMAN_MODE, ]; - static readonly doorModes = [ - RomiCore.LiftRequest.DOOR_CLOSED, - RomiCore.LiftRequest.DOOR_OPEN, - ] + static readonly doorModes = [RomiCore.LiftRequest.DOOR_CLOSED, RomiCore.LiftRequest.DOOR_OPEN]; static getLiftRequestModes(): requestManagerModes { let modes: requestManagerModes = {}; this.liftRequestModes.forEach(element => { const key = this.requestModeToString(element); - modes[key] = element + modes[key] = element; }); - return modes + return modes; } static getDoorModes(): requestManagerModes { let modes: requestManagerModes = {}; this.doorModes.forEach(element => { const key = this.doorStateToString(element); - modes[key] = element + modes[key] = element; }); - return modes + return modes; } static requestModeToString(requestMode: number): string { diff --git a/src/mock/fake-transport.ts b/src/mock/fake-transport.ts index d72b0a9a8..dbc444c86 100644 --- a/src/mock/fake-transport.ts +++ b/src/mock/fake-transport.ts @@ -49,6 +49,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const liftStates = fakeLiftStates(); timer = window.setInterval(() => { for (const state of Object.values(liftStates)) { + debug('publishing lift state'); cb(state as Message); } }, 1000); diff --git a/src/stories/baseComponents/lift-panel.tsx b/src/stories/baseComponents/lift-panel.tsx deleted file mode 100644 index 444e397b0..000000000 --- a/src/stories/baseComponents/lift-panel.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import React from 'react'; - -import { LiftItem } from '../../components/lift-item/lift-item'; -import { LiftsPanelProps } from '../../components/lift-item/lifts-panel'; - -export default function LiftButton(props: LiftsPanelProps) { - const { lifts, liftStates } = props; - const liftRefs = React.useRef>({}); - - return ( - - {lifts.map(lift => { - const liftState = liftStates[lift.name]; - return ( - (liftRefs.current[lift.name] = ref)} - liftState={liftState} - /> - ); - })} - - ); -} diff --git a/src/stories/door.stories.tsx b/src/stories/door.stories.tsx index d313626c9..5f35e1518 100644 --- a/src/stories/door.stories.tsx +++ b/src/stories/door.stories.tsx @@ -2,10 +2,8 @@ import { Divider, Typography } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React from 'react'; import DoorsPanel from '../components/doors-panel'; -import DoorStateManager from '../door-state-manager'; -import FakeTransport from '../mock/fake-transport'; import DoorComponent from './baseComponents/door-component'; -// import DoorButton from './baseComponents/door-panel'; + import { componentDisplayStyle, defaultStyles, diff --git a/src/stories/lift.stories.tsx b/src/stories/lift.stories.tsx index 84e85c059..744e5fc93 100644 --- a/src/stories/lift.stories.tsx +++ b/src/stories/lift.stories.tsx @@ -1,15 +1,14 @@ -import React from 'react'; import { Divider, Typography } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; - +import React from 'react'; +import LiftsPanel from '../components/lift-item/lifts-panel'; import LiftComponent from './baseComponents/lift-component'; -import LiftButton from './baseComponents/lift-panel'; import { + componentDisplayStyle, + defaultStyles, lift, lifts, liftStates, - componentDisplayStyle, - defaultStyles, StyleTyping, } from './baseComponents/utils'; @@ -171,6 +170,6 @@ export const liftPanel = () => ( Lift State Button color and representation - + ); From 4647ef37cc32914f19971626a8cb687ad147296b Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 12:37:01 +0800 Subject: [PATCH 04/16] optimize dispenser panel --- src/components/app.tsx | 19 +- src/components/dispenser-item.tsx | 192 +++++++++--------- src/components/dispensers-panel.tsx | 52 +++-- src/dispenser-state-manager.ts | 14 +- src/mock/fake-transport.ts | 1 + .../baseComponents/dispenser-panel.tsx | 24 --- src/stories/design-decision.stories.tsx | 11 +- src/stories/dispenser.stories.tsx | 4 +- 8 files changed, 163 insertions(+), 154 deletions(-) delete mode 100644 src/stories/baseComponents/dispenser-panel.tsx diff --git a/src/components/app.tsx b/src/components/app.tsx index 462e0ed7c..b0f44c36b 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -29,7 +29,7 @@ import { LiftStateContext } from './schedule-visualizer/lift-overlay'; import SettingsDrawer from './settings-drawer'; import { SpotlightValue } from './spotlight-value'; -const debug = Debug('log'); +const debug = Debug('App'); const borderRadius = 20; const useStyles = makeStyles(theme => ({ @@ -112,6 +112,8 @@ function makeViewMap(): ViewMap { const viewMap = makeViewMap(); export default function App(props: AppProps): JSX.Element { + debug('render'); + const classes = useStyles(); const { transportFactory, trajectoryManagerFactory, appResources } = props.appConfig; const [transport, setTransport] = React.useState(undefined); @@ -178,6 +180,9 @@ export default function App(props: AppProps): JSX.Element { fleetManager.on('updated', () => setFleets(fleetManager.fleets())); liftStateManager.on('updated', () => setLiftStates(liftStateManager.liftStates())); doorStateManager.on('updated', () => setDoorStates(doorStateManager.doorStates())); + dispenserStateManager.on('updated', () => + setDispenserStates(dispenserStateManager.dispenserStates()), + ); setTransport(x); }) .catch((e: CloseEvent) => { @@ -220,18 +225,6 @@ export default function App(props: AppProps): JSX.Element { })(); }, [appResources]); - React.useEffect(() => { - if (currentView === OmniPanelViewIndex.Dispensers) { - const listener = () => setDispenserStates(dispenserStateManager.dispenserStates()); - dispenserStateManager.on('updated', listener); - debug('started tracking dispenser states'); - return () => { - dispenserStateManager.off('updated', listener); - debug('stopped tracking dispenser states'); - }; - } - }, [currentView, dispenserStateManager]); - React.useEffect(() => { setDoors(buildingMap ? buildingMap.levels.flatMap(x => x.doors) : []); setLifts(buildingMap ? buildingMap.lifts : []); diff --git a/src/components/dispenser-item.tsx b/src/components/dispenser-item.tsx index d5d6cb80f..ee144a22b 100644 --- a/src/components/dispenser-item.tsx +++ b/src/components/dispenser-item.tsx @@ -4,116 +4,122 @@ import { ExpansionPanelDetails, ExpansionPanelProps, ExpansionPanelSummary, - makeStyles, - Typography, List, ListItem, + makeStyles, + Typography, } from '@material-ui/core'; import { CSSProperties } from '@material-ui/core/styles/withStyles'; import { ExpandMore as ExpandMoreIcon } from '@material-ui/icons'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; - +import { colorPalette } from '../util/css-utils'; import DisableableTypography from './disableable-typography'; import OmniPanelStatusLabels from './omni-panel-status-labels'; -import { colorPalette } from '../util/css-utils'; + +const debug = Debug('DispenserItem'); export interface DispenserItemProps extends Omit { dispenserState: Readonly; } -export const DispenserItem = React.forwardRef(function( - props: DispenserItemProps, - ref: React.Ref, -): React.ReactElement { - const { dispenserState, ...otherProps } = props; - const classes = useStyles(); - const dispenserModeLabelClasses = useDispenserModeLabelStyles(); - - function dispenserModeLabelClass(): string { - switch (dispenserState.mode) { - case RomiCore.DispenserState.IDLE: - return `${classes.dispenserLabel} ${dispenserModeLabelClasses.idle}`; - case RomiCore.DispenserState.BUSY: - return `${classes.dispenserLabel} ${dispenserModeLabelClasses.busy}`; - case RomiCore.DispenserState.OFFLINE: - return `${classes.dispenserLabel} ${dispenserModeLabelClasses.offline}`; - default: - return `${classes.dispenserLabel} ${dispenserModeLabelClasses.unknown}`; +export const DispenserItem = React.memo( + React.forwardRef(function( + props: DispenserItemProps, + ref: React.Ref, + ): React.ReactElement { + debug('render'); + + const { dispenserState, ...otherProps } = props; + const classes = useStyles(); + const dispenserModeLabelClasses = useDispenserModeLabelStyles(); + + function dispenserModeLabelClass(): string { + switch (dispenserState.mode) { + case RomiCore.DispenserState.IDLE: + return `${classes.dispenserLabel} ${dispenserModeLabelClasses.idle}`; + case RomiCore.DispenserState.BUSY: + return `${classes.dispenserLabel} ${dispenserModeLabelClasses.busy}`; + case RomiCore.DispenserState.OFFLINE: + return `${classes.dispenserLabel} ${dispenserModeLabelClasses.offline}`; + default: + return `${classes.dispenserLabel} ${dispenserModeLabelClasses.unknown}`; + } } - } - - function dispenserRequestQueueId(): React.ReactElement { - if (dispenserState.request_guid_queue.length === 0) { - return ( - - Unknown - - ); - } else { - return ( - - {dispenserState.request_guid_queue.map(id => ( - - {id} - - ))} - - ); + + function dispenserRequestQueueId(): React.ReactElement { + if (dispenserState.request_guid_queue.length === 0) { + return ( + + Unknown + + ); + } else { + return ( + + {dispenserState.request_guid_queue.map(id => ( + + {id} + + ))} + + ); + } } - } - - function dispenserModeToString(): string { - switch (dispenserState.mode) { - case RomiCore.DispenserState.IDLE: - return 'IDLE'; - case RomiCore.DispenserState.BUSY: - return 'ONLINE'; - case RomiCore.DispenserState.OFFLINE: - return 'OFFLINE'; - default: - return 'N/A'; + + function dispenserModeToString(): string { + switch (dispenserState.mode) { + case RomiCore.DispenserState.IDLE: + return 'IDLE'; + case RomiCore.DispenserState.BUSY: + return 'ONLINE'; + case RomiCore.DispenserState.OFFLINE: + return 'OFFLINE'; + default: + return 'N/A'; + } } - } - - return ( - - } - > - - - -
- Name: - {dispenserState.guid} -
- -
- No. Queued Requests: - - {String(dispenserState.request_guid_queue.length)} - -
- -
- Request Queue ID: - {dispenserRequestQueueId()} -
- -
- Seconds Remaining: - {String(dispenserState.seconds_remaining)} -
- - - ); -}); + + return ( + + } + > + + + +
+ Name: + {dispenserState.guid} +
+ +
+ No. Queued Requests: + + {String(dispenserState.request_guid_queue.length)} + +
+ +
+ Request Queue ID: + {dispenserRequestQueueId()} +
+ +
+ Seconds Remaining: + {String(dispenserState.seconds_remaining)} +
+ + + ); + }), +); export default DispenserItem; diff --git a/src/components/dispensers-panel.tsx b/src/components/dispensers-panel.tsx index 02c5aa5b5..fa218e2de 100644 --- a/src/components/dispensers-panel.tsx +++ b/src/components/dispensers-panel.tsx @@ -1,18 +1,48 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; -import DispenserItem from './dispenser-item'; +import DispenserItem, { DispenserItemProps } from './dispenser-item'; import { SpotlightValue } from './spotlight-value'; +const debug = Debug('DispenserPanel'); + export interface DispenserPanelProps { dispenserStates: Readonly>; spotlight?: Readonly>; } -export default function DispenserPanel(props: DispenserPanelProps): JSX.Element { - const { spotlight } = props; +export const DispenserPanel = React.memo((props: DispenserPanelProps) => { + debug('render'); + + const { dispenserStates, spotlight } = props; const dispenserRefs = React.useRef>({}); const [expanded, setExpanded] = React.useState>({}); + const storeRef = React.useCallback((ref: HTMLElement | null) => { + if (!ref) { + return; + } + const guid = ref.getAttribute('data-guid'); + if (!guid) { + return; + } + dispenserRefs.current[guid] = ref; + }, []); + + const onChange = React.useCallback['onChange']>( + (event, newExpanded) => { + const guid = (event.currentTarget as HTMLElement).parentElement?.getAttribute('data-guid'); + if (!guid) { + return; + } + setExpanded(prev => ({ + ...prev, + [guid]: newExpanded, + })); + }, + [], + ); + React.useEffect(() => { if (!spotlight) { return; @@ -28,23 +58,21 @@ export default function DispenserPanel(props: DispenserPanelProps): JSX.Element ref.scrollIntoView({ behavior: 'smooth' }); }, [spotlight]); - const listItems = Object.keys(props.dispenserStates).map(guid => { + const listItems = Object.keys(dispenserStates).map((guid, i) => { const state = props.dispenserStates[guid]; return ( (dispenserRefs.current[state.guid] = ref)} + data-guid={state.guid} + ref={storeRef} dispenserState={state} expanded={Boolean(expanded[state.guid])} - onChange={(_, newExpanded) => - setExpanded(prev => ({ - ...prev, - [state.guid]: newExpanded, - })) - } + onChange={onChange} /> ); }); return {listItems}; -} +}); + +export default DispenserPanel; diff --git a/src/dispenser-state-manager.ts b/src/dispenser-state-manager.ts index cf28c1452..10a1a0509 100644 --- a/src/dispenser-state-manager.ts +++ b/src/dispenser-state-manager.ts @@ -11,11 +11,17 @@ export default class DispenserStateManager extends EventEmitter { } startSubscription(transport: RomiCore.Transport) { - transport.subscribe(RomiCore.dispenserStates, dispenserState => { - this._dispenserStates[dispenserState.guid] = dispenserState; - this.emit('updated'); - }); + this._subscriptions.push( + transport.subscribe(RomiCore.dispenserStates, dispenserState => { + this._dispenserStates[dispenserState.guid] = dispenserState; + this.emit('updated'); + }), + ); + } + stopAllSubscriptions(): void { + this._subscriptions.forEach(sub => sub.unsubscribe()); } private _dispenserStates: Record = {}; + private _subscriptions: RomiCore.Subscription[] = []; } diff --git a/src/mock/fake-transport.ts b/src/mock/fake-transport.ts index dbc444c86..a3f7a8972 100644 --- a/src/mock/fake-transport.ts +++ b/src/mock/fake-transport.ts @@ -68,6 +68,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const dispenserStates = fakeDispenserStates(); timer = window.setInterval(() => { for (const state of Object.values(dispenserStates)) { + debug('publishing dispenser state'); cb(state as Message); } }, 1000); diff --git a/src/stories/baseComponents/dispenser-panel.tsx b/src/stories/baseComponents/dispenser-panel.tsx deleted file mode 100644 index fba257145..000000000 --- a/src/stories/baseComponents/dispenser-panel.tsx +++ /dev/null @@ -1,24 +0,0 @@ -import React from 'react'; - -import { DispenserItem } from '../../components/dispenser-item'; -import { DispenserPanelProps } from '../../components/dispensers-panel'; - -export default function DispenserButton(props: DispenserPanelProps) { - const { dispenserStates } = props; - const dispenserRefs = React.useRef>({}); - - return ( - - {Object.keys(dispenserStates).map(guid => { - const state = dispenserStates[guid]; - return ( - (dispenserRefs.current[state.guid] = ref)} - dispenserState={state} - /> - ); - })} - - ); -} diff --git a/src/stories/design-decision.stories.tsx b/src/stories/design-decision.stories.tsx index 0e2cd5440..3335eb347 100644 --- a/src/stories/design-decision.stories.tsx +++ b/src/stories/design-decision.stories.tsx @@ -1,8 +1,7 @@ -import React from 'react'; import { Divider, Typography } from '@material-ui/core'; - -import { StyleTyping, defaultStyles, dispenserStates } from './baseComponents/utils'; -import DispenserButton from './baseComponents/dispenser-panel'; +import React from 'react'; +import DispenserPanel from '../components/dispensers-panel'; +import { defaultStyles, dispenserStates, StyleTyping } from './baseComponents/utils'; export default { title: 'Design Decisions', @@ -27,7 +26,7 @@ export const handleLongName = () => (
- +
); @@ -43,7 +42,7 @@ export const handleUnknown = () => (
- +
); diff --git a/src/stories/dispenser.stories.tsx b/src/stories/dispenser.stories.tsx index 22e135bf7..0afbb8131 100644 --- a/src/stories/dispenser.stories.tsx +++ b/src/stories/dispenser.stories.tsx @@ -1,8 +1,8 @@ import React from 'react'; import { Typography } from '@material-ui/core'; -import DispenserButton from './baseComponents/dispenser-panel'; import { dispenserStates, defaultStyles, StyleTyping } from './baseComponents/utils'; +import DispenserPanel from '../components/dispensers-panel'; export default { title: 'Dispenser', @@ -23,6 +23,6 @@ export const dispenserPanel = () => ( Dispenser State Button color and representation - + ); From d1ac0e9bed76cb3f8e14f1aed5914824a6f5df51 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 13:09:13 +0800 Subject: [PATCH 05/16] optimize robot panel --- src/components/robot-item.tsx | 59 ++++++++++++---------- src/components/robots-panel.tsx | 58 ++++++++++++++------- src/fleet-manager.ts | 17 +++++-- src/mock/fake-transport.ts | 16 ++---- src/stories/baseComponents/robot-panel.tsx | 26 ---------- src/stories/robot.stories.tsx | 4 +- 6 files changed, 92 insertions(+), 88 deletions(-) delete mode 100644 src/stories/baseComponents/robot-panel.tsx diff --git a/src/components/robot-item.tsx b/src/components/robot-item.tsx index 2510f520d..f5d13155f 100644 --- a/src/components/robot-item.tsx +++ b/src/components/robot-item.tsx @@ -6,11 +6,13 @@ import { makeStyles, } from '@material-ui/core'; import { ExpandMore as ExpandMoreIcon } from '@material-ui/icons'; -import { RobotInformation } from './robot-item-information'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; - import OmniPanelStatusLabels from './omni-panel-status-labels'; +import { RobotInformation } from './robot-item-information'; + +const debug = Debug('RobotItem'); export interface RobotItemProps extends Omit { fleetName: string; @@ -18,30 +20,35 @@ export interface RobotItemProps extends Omit { onRobotClick?(robot: RomiCore.RobotState): void; } -export const RobotItem = React.forwardRef(function( - props: RobotItemProps, - ref: React.Ref, -): React.ReactElement { - const { robot, onRobotClick, fleetName, ...otherProps } = props; - const classes = useStyles(); - return ( - - } - > - - - - - - - ); -}); +export const RobotItem = React.memo( + React.forwardRef(function( + props: RobotItemProps, + ref: React.Ref, + ): React.ReactElement { + const { robot, onRobotClick, fleetName, ...otherProps } = props; + const classes = useStyles(); + + debug('render %s', robot.name); + + return ( + + } + > + + + + + + + ); + }), +); export default RobotItem; diff --git a/src/components/robots-panel.tsx b/src/components/robots-panel.tsx index 50b6c068a..fe7d1f62d 100644 --- a/src/components/robots-panel.tsx +++ b/src/components/robots-panel.tsx @@ -1,19 +1,45 @@ -import { SpotlightValue } from './spotlight-value'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React from 'react'; -import RobotItem from './robot-item'; +import RobotItem, { RobotItemProps } from './robot-item'; +import { SpotlightValue } from './spotlight-value'; export interface RobotsPanelProps { fleets: readonly RomiCore.FleetState[]; spotlight?: Readonly>; - onRobotClick?(robot: RomiCore.RobotState): void; } -export default function RobotsPanel(props: RobotsPanelProps): React.ReactElement { - const { fleets, spotlight, onRobotClick } = props; +export const RobotsPanel = React.memo((props: RobotsPanelProps) => { + const { fleets, spotlight } = props; const robotRefs = React.useRef>({}); const [expanded, setExpanded] = React.useState>>({}); + const storeRef = React.useCallback((ref: HTMLElement | null) => { + if (!ref) { + return; + } + const guid = ref.getAttribute('data-guid'); + if (!guid) { + return; + } + robotRefs.current[guid] = ref; + }, []); + + const onChange = React.useCallback['onChange']>((event, newExpanded) => { + const fleetName = (event.currentTarget as HTMLElement).parentElement?.getAttribute( + 'data-fleet', + ); + const robotName = (event.currentTarget as HTMLElement).parentElement?.getAttribute('data-name'); + if (!fleetName || !robotName) { + return; + } + setExpanded(prev => ({ + ...prev, + [`${fleetName}-${robotName}`]: newExpanded, + })); + }, []); + + const transitionProps = React.useMemo(() => ({ unmountOnExit: true }), []); + React.useEffect(() => { if (!spotlight) { return; @@ -34,22 +60,20 @@ export default function RobotsPanel(props: RobotsPanelProps): React.ReactElement {fleets.flatMap(fleet => fleet.robots.map(robot => ( (robotRefs.current[robot.name] = ref)} + key={`${fleet.name}-${robot.name}`} + data-fleet={fleet.name} + data-name={robot.name} + ref={storeRef} fleetName={fleet.name} robot={robot} - onClick={() => onRobotClick && onRobotClick(robot)} - expanded={Boolean(expanded[robot.name])} - onChange={(_, newExpanded) => - setExpanded(prev => ({ - ...prev, - [robot.name]: newExpanded, - })) - } - TransitionProps={{ unmountOnExit: true }} + expanded={Boolean(expanded[`${fleet.name}-${robot.name}`])} + onChange={onChange} + TransitionProps={transitionProps} /> )), )} ); -} +}); + +export default RobotsPanel; diff --git a/src/fleet-manager.ts b/src/fleet-manager.ts index 5b6b8f940..29be0fa52 100644 --- a/src/fleet-manager.ts +++ b/src/fleet-manager.ts @@ -3,7 +3,7 @@ import EventEmitter from 'eventemitter3'; type Events = { updated: []; -} +}; export default class FleetManager extends EventEmitter { fleets(): RomiCore.FleetState[] { @@ -11,11 +11,18 @@ export default class FleetManager extends EventEmitter { } startSubscription(transport: RomiCore.Transport) { - transport.subscribe(RomiCore.fleetStates, fleetState => { - this._fleets[fleetState.name] = fleetState; - this.emit('updated'); - }); + this._subscriptions.push( + transport.subscribe(RomiCore.fleetStates, fleetState => { + this._fleets[fleetState.name] = fleetState; + this.emit('updated'); + }), + ); + } + + stopAllSubscriptions(): void { + this._subscriptions.forEach(sub => sub.unsubscribe()); } private _fleets: Record = {}; + private _subscriptions: RomiCore.Subscription[] = []; } diff --git a/src/mock/fake-transport.ts b/src/mock/fake-transport.ts index a3f7a8972..aa9c369c1 100644 --- a/src/mock/fake-transport.ts +++ b/src/mock/fake-transport.ts @@ -39,7 +39,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const doorStates = this._doorStateFactory(); timer = window.setInterval(() => { for (const state of Object.values(doorStates)) { - debug('publishing door state'); + debug('publishing door state, %s', state.door_name); cb(state as Message); } }, 1000); @@ -49,7 +49,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const liftStates = fakeLiftStates(); timer = window.setInterval(() => { for (const state of Object.values(liftStates)) { - debug('publishing lift state'); + debug('publishing lift state, %s', state.lift_name); cb(state as Message); } }, 1000); @@ -59,6 +59,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const fleets = fakeFleets(); timer = window.setInterval(() => { for (const fleet of fleets) { + debug('publishing fleet state, %s', fleet.name); cb(fleet as Message); } }, 1000); @@ -68,7 +69,7 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. const dispenserStates = fakeDispenserStates(); timer = window.setInterval(() => { for (const state of Object.values(dispenserStates)) { - debug('publishing dispenser state'); + debug('publishing dispenser state, %s', state.guid); cb(state as Message); } }, 1000); @@ -76,15 +77,6 @@ export class FakeTransport extends RomiCore.TransportEvents implements RomiCore. } } - if (topic !== RomiCore.liftStates && topic.topic === 'lift_states') { - const liftStates = fakeLiftStates(); - timer = window.setInterval(() => { - for (const state of Object.values(liftStates)) { - cb(state as Message); - } - }, 1000); - } - return { unsubscribe: () => { clearInterval(timer); diff --git a/src/stories/baseComponents/robot-panel.tsx b/src/stories/baseComponents/robot-panel.tsx deleted file mode 100644 index 201ec30ec..000000000 --- a/src/stories/baseComponents/robot-panel.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import React from 'react'; - -import { RobotItem } from '../../components/robot-item'; -import { RobotsPanelProps } from '../../components/robots-panel'; - -export default function RobotButton(props: RobotsPanelProps) { - const { fleets } = props; - const robotRefs = React.useRef>({}); - - return ( - - {fleets.flatMap(fleet => { - return fleet.robots.map(robot => { - return ( - (robotRefs.current[robot.name] = ref)} - fleetName={fleet.name} - robot={robot} - /> - ); - }); - })} - - ); -} diff --git a/src/stories/robot.stories.tsx b/src/stories/robot.stories.tsx index 37d458575..f678fa5f1 100644 --- a/src/stories/robot.stories.tsx +++ b/src/stories/robot.stories.tsx @@ -10,8 +10,8 @@ import { defaultStyles, StyleTyping, } from './baseComponents/utils'; -import RobotButton from './baseComponents/robot-panel'; import RobotGallery from './baseComponents/robot-gallery'; +import RobotsPanel from '../components/robots-panel'; export default { title: 'Robot', @@ -90,7 +90,7 @@ export const robotPanel = () => ( Robot State Button color and representation - + ); From b8a849fc19f62a5a4c2ab1a1f03884c942471e65 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 14:21:16 +0800 Subject: [PATCH 06/16] restore onrobotclick functionality --- src/components/robots-panel.tsx | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/components/robots-panel.tsx b/src/components/robots-panel.tsx index fe7d1f62d..4c1a7d1f7 100644 --- a/src/components/robots-panel.tsx +++ b/src/components/robots-panel.tsx @@ -6,10 +6,11 @@ import { SpotlightValue } from './spotlight-value'; export interface RobotsPanelProps { fleets: readonly RomiCore.FleetState[]; spotlight?: Readonly>; + onRobotClick?(robot: RomiCore.RobotState): void; } export const RobotsPanel = React.memo((props: RobotsPanelProps) => { - const { fleets, spotlight } = props; + const { fleets, spotlight, onRobotClick } = props; const robotRefs = React.useRef>({}); const [expanded, setExpanded] = React.useState>>({}); @@ -38,6 +39,23 @@ export const RobotsPanel = React.memo((props: RobotsPanelProps) => { })); }, []); + const onClick = React.useRef['onClick']>(event => { + const fleetName = (event.currentTarget as HTMLElement).getAttribute('data-fleet'); + const robotName = (event.currentTarget as HTMLElement).getAttribute('data-name'); + if (!fleetName || !robotName) { + return; + } + const fleet = fleets.find(f => f.name === fleetName); + if (!fleet) { + return; + } + const robot = fleet.robots.find(r => r.name === robotName); + if (!robot) { + return; + } + onRobotClick && onRobotClick(robot); + }); + const transitionProps = React.useMemo(() => ({ unmountOnExit: true }), []); React.useEffect(() => { @@ -66,6 +84,7 @@ export const RobotsPanel = React.memo((props: RobotsPanelProps) => { ref={storeRef} fleetName={fleet.name} robot={robot} + onClick={onClick.current} expanded={Boolean(expanded[`${fleet.name}-${robot.name}`])} onChange={onChange} TransitionProps={transitionProps} From 51beff3bb91852da35028631a9adfabaa2c2aa1c Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 16:26:33 +0800 Subject: [PATCH 07/16] group some of the contexts to reduce the mess in app.tsx --- src/app-contexts.ts | 8 - src/components/app-contexts.tsx | 25 +++ src/components/app.tsx | 195 +++++++++--------- src/components/rmf-contexts.tsx | 19 ++ .../schedule-visualizer/doors-overlay.tsx | 2 +- src/components/schedule-visualizer/index.tsx | 10 +- .../schedule-visualizer/lift-overlay.tsx | 7 +- src/components/schedule-visualizer/robot.tsx | 2 +- src/settings.ts | 4 - src/stories/baseComponents/robot-gallery.tsx | 2 +- 10 files changed, 149 insertions(+), 125 deletions(-) delete mode 100644 src/app-contexts.ts create mode 100644 src/components/app-contexts.tsx create mode 100644 src/components/rmf-contexts.tsx diff --git a/src/app-contexts.ts b/src/app-contexts.ts deleted file mode 100644 index c642a4f14..000000000 --- a/src/app-contexts.ts +++ /dev/null @@ -1,8 +0,0 @@ -import * as RomiCore from '@osrf/romi-js-core-interfaces'; -import { createContext } from 'react'; -import { ResourceConfigurationsType } from './resource-manager'; - -/* Declares the ResourcesContext which contains the resources used on the app*/ -export const ResourcesContext = createContext({}); - -export const DoorStateContext = createContext>({}); diff --git a/src/components/app-contexts.tsx b/src/components/app-contexts.tsx new file mode 100644 index 000000000..f5ddc40b4 --- /dev/null +++ b/src/components/app-contexts.tsx @@ -0,0 +1,25 @@ +import React from 'react'; +import { ResourceConfigurationsType } from '../resource-manager'; +import { defaultSettings, Settings } from '../settings'; +import { NotificationBarContext, NotificationBarProps } from './notification-bar'; + +/* Declares the ResourcesContext which contains the resources used on the app*/ +export const ResourcesContext = React.createContext({}); + +export const SettingsContext = React.createContext(defaultSettings()); + +export interface AppContextProviderProps extends React.PropsWithChildren<{}> { + settings: Settings; + notificationDispatch: React.Dispatch>; +} + +export function AppContextProvider(props: AppContextProviderProps): React.ReactElement { + const { settings, notificationDispatch, children } = props; + return ( + + + {children} + + + ); +} diff --git a/src/components/app.tsx b/src/components/app.tsx index b0f44c36b..8d63e9289 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -5,14 +5,14 @@ import Debug from 'debug'; import React from 'react'; import 'typeface-roboto'; import { AppConfig } from '../app-config'; -import { DoorStateContext } from '../app-contexts'; import DispenserStateManager from '../dispenser-state-manager'; import DoorStateManager from '../door-state-manager'; import FleetManager from '../fleet-manager'; import LiftStateManager from '../lift-state-manager'; import { ResourceConfigurationsType } from '../resource-manager'; import { RobotTrajectoryManager } from '../robot-trajectory-manager'; -import { loadSettings, saveSettings, Settings, SettingsContext } from '../settings'; +import { loadSettings, saveSettings, Settings } from '../settings'; +import { AppContextProvider } from './app-contexts'; import './app.css'; import CommandsPanel from './commands-panel'; import DispensersPanel from './dispensers-panel'; @@ -20,12 +20,12 @@ import DoorsPanel from './doors-panel'; import LiftsPanel from './lift-item/lifts-panel'; import LoadingScreen, { LoadingScreenProps } from './loading-screen'; import MainMenu from './main-menu'; -import NotificationBar, { NotificationBarContext, NotificationBarProps } from './notification-bar'; +import NotificationBar, { NotificationBarProps } from './notification-bar'; import OmniPanel from './omni-panel'; import OmniPanelView from './omni-panel-view'; +import { RmfContextProvider } from './rmf-contexts'; import RobotsPanel from './robots-panel'; import ScheduleVisualizer from './schedule-visualizer'; -import { LiftStateContext } from './schedule-visualizer/lift-overlay'; import SettingsDrawer from './settings-drawer'; import { SpotlightValue } from './spotlight-value'; @@ -291,103 +291,96 @@ export default function App(props: AppProps): JSX.Element { } return ( - - - - - {loading && } -
- - - - Dashboard - - setShowOmniPanel(!showOmniPanel)}> - - - setShowSettings(true)}> - - - - - {buildingMap && ( - - )} - - - - - - - - - - - - - - - - - - - - - - - { - setSettings(newSettings); - saveSettings(newSettings); - }} - onClose={() => setShowSettings(false)} - /> -
- + + {loading && } +
+ + + + Dashboard + + setShowOmniPanel(!showOmniPanel)}> + + + setShowSettings(true)}> + + + + + {buildingMap && ( + - - - - + )} + + + + + + + + + + + + + + + + + + + + + + + { + setSettings(newSettings); + saveSettings(newSettings); + }} + onClose={() => setShowSettings(false)} + /> +
+ + + ); } diff --git a/src/components/rmf-contexts.tsx b/src/components/rmf-contexts.tsx new file mode 100644 index 000000000..0e2adb67c --- /dev/null +++ b/src/components/rmf-contexts.tsx @@ -0,0 +1,19 @@ +import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import React from 'react'; + +export const DoorStateContext = React.createContext>({}); +export const LiftStateContext = React.createContext>({}); + +export interface RmfContextProviderProps extends React.PropsWithChildren<{}> { + doorStates: Record; + liftStates: Record; +} + +export function RmfContextProvider(props: RmfContextProviderProps): React.ReactElement { + const { doorStates, liftStates, children } = props; + return ( + + {children} + + ); +} diff --git a/src/components/schedule-visualizer/doors-overlay.tsx b/src/components/schedule-visualizer/doors-overlay.tsx index c3e8a79b9..861ba5239 100644 --- a/src/components/schedule-visualizer/doors-overlay.tsx +++ b/src/components/schedule-visualizer/doors-overlay.tsx @@ -5,8 +5,8 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React, { useContext } from 'react'; -import { DoorStateContext } from '../../app-contexts'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; +import { DoorStateContext } from '../rmf-contexts'; import Door from './door/door'; import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; diff --git a/src/components/schedule-visualizer/index.tsx b/src/components/schedule-visualizer/index.tsx index e25eaf733..f894baa90 100644 --- a/src/components/schedule-visualizer/index.tsx +++ b/src/components/schedule-visualizer/index.tsx @@ -3,6 +3,7 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; import * as L from 'leaflet'; import React from 'react'; import { AttributionControl, ImageOverlay, LayersControl, Map as LMap, Pane } from 'react-leaflet'; +import { ResourceConfigurationsType } from '../../resource-manager'; import { Conflict, DefaultTrajectoryManager, @@ -10,9 +11,12 @@ import { Trajectory, TrajectoryResponse, } from '../../robot-trajectory-manager'; -import { AnimationSpeed, SettingsContext, TrajectoryAnimation } from '../../settings'; +import { AnimationSpeed, TrajectoryAnimation } from '../../settings'; import { toBlobUrl } from '../../util'; +import { ResourcesContext, SettingsContext } from '../app-contexts'; import ColorManager from './colors'; +import DoorsOverlay from './doors-overlay'; +import LiftsOverlay from './lift-overlay'; import RobotTrajectoriesOverlay, { RobotTrajectoryContext } from './robot-trajectories-overlay'; import RobotTrajectory from './robot-trajectory'; import RobotsOverlay from './robots-overlay'; @@ -21,10 +25,6 @@ import { withFollowAnimation, withOutlineAnimation, } from './trajectory-animations'; -import DoorsOverlay from './doors-overlay'; -import LiftsOverlay from './lift-overlay'; -import { ResourcesContext } from '../../app-contexts'; -import { ResourceConfigurationsType } from '../../resource-manager'; import WaypointsOverlay from './waypoints-overlay'; const useStyles = makeStyles(() => ({ diff --git a/src/components/schedule-visualizer/lift-overlay.tsx b/src/components/schedule-visualizer/lift-overlay.tsx index 04dee7598..eabd9e2f7 100644 --- a/src/components/schedule-visualizer/lift-overlay.tsx +++ b/src/components/schedule-visualizer/lift-overlay.tsx @@ -1,8 +1,9 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; -import React, { createContext, useContext } from 'react'; +import React, { useContext } from 'react'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; -import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; +import { LiftStateContext } from '../rmf-contexts'; import Lift from './lift'; +import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; export interface LiftsOverlayProps extends SVGOverlayProps { currentFloor: string; @@ -33,5 +34,3 @@ export default function LiftsOverlay(props: LiftsOverlayProps): React.ReactEleme ); } - -export const LiftStateContext = createContext>>({}); diff --git a/src/components/schedule-visualizer/robot.tsx b/src/components/schedule-visualizer/robot.tsx index 86a3d4268..545e0c9c7 100644 --- a/src/components/schedule-visualizer/robot.tsx +++ b/src/components/schedule-visualizer/robot.tsx @@ -1,7 +1,7 @@ import { makeStyles } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React, { useContext, useState } from 'react'; -import { ResourcesContext } from '../../app-contexts'; +import { ResourcesContext } from '../app-contexts'; import ResourceManager from '../../resource-manager'; import ColorManager from './colors'; import RobotDefaultIcon from './robot-default-icon'; diff --git a/src/settings.ts b/src/settings.ts index 9f030931b..83e8fe71c 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -1,5 +1,3 @@ -import React from 'react'; - export enum TrajectoryAnimation { None, Fill, @@ -36,5 +34,3 @@ export function defaultSettings(): Settings { trajectoryAnimationSpeed: AnimationSpeed.Normal, }; } - -export const SettingsContext = React.createContext(defaultSettings()); diff --git a/src/stories/baseComponents/robot-gallery.tsx b/src/stories/baseComponents/robot-gallery.tsx index 5bb378e93..e750a838b 100644 --- a/src/stories/baseComponents/robot-gallery.tsx +++ b/src/stories/baseComponents/robot-gallery.tsx @@ -3,7 +3,7 @@ import Paper from '@material-ui/core/Paper'; import Typography from '@material-ui/core/Typography'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React from 'react'; -import { ResourcesContext } from '../../app-contexts'; +import { ResourcesContext } from '../../components/app-contexts'; import ColorManager from '../../components/schedule-visualizer/colors'; import Robot, { RobotProps } from '../../components/schedule-visualizer/robot'; import { ResourceConfigurationsType } from '../../resource-manager'; From a1a26755b24229dd25be43fa83f81c06a9ec80c2 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Wed, 2 Sep 2020 16:35:50 +0800 Subject: [PATCH 08/16] fix robot spotlight --- src/components/app.tsx | 4 ++-- src/components/robots-panel.tsx | 7 ++++--- src/components/schedule-visualizer/index.tsx | 2 +- .../schedule-visualizer/robots-overlay.tsx | 17 ++++++++++------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index 8d63e9289..e04100860 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -236,10 +236,10 @@ export default function App(props: AppProps): JSX.Element { setDoorSpotlight({ value: door.name }); } - function handleRobotClick(robot: RomiCore.RobotState): void { + function handleRobotClick(fleet: string, robot: RomiCore.RobotState): void { setShowOmniPanel(true); setCurrentView(OmniPanelViewIndex.Robots); - setRobotSpotlight({ value: robot.name }); + setRobotSpotlight({ value: `${fleet}-${robot.name}` }); } function handleLiftClick(lift: RomiCore.Lift): void { diff --git a/src/components/robots-panel.tsx b/src/components/robots-panel.tsx index 4c1a7d1f7..77a0d147b 100644 --- a/src/components/robots-panel.tsx +++ b/src/components/robots-panel.tsx @@ -18,11 +18,12 @@ export const RobotsPanel = React.memo((props: RobotsPanelProps) => { if (!ref) { return; } - const guid = ref.getAttribute('data-guid'); - if (!guid) { + const fleetName = ref.getAttribute('data-fleet'); + const robotName = ref.getAttribute('data-name'); + if (!fleetName || !robotName) { return; } - robotRefs.current[guid] = ref; + robotRefs.current[`${fleetName}-${robotName}`] = ref; }, []); const onChange = React.useCallback['onChange']>((event, newExpanded) => { diff --git a/src/components/schedule-visualizer/index.tsx b/src/components/schedule-visualizer/index.tsx index f894baa90..9aad0aeba 100644 --- a/src/components/schedule-visualizer/index.tsx +++ b/src/components/schedule-visualizer/index.tsx @@ -49,7 +49,7 @@ export interface ScheduleVisualizerProps { appResources?: Readonly; onDoorClick?(door: RomiCore.Door): void; onLiftClick?(lift: RomiCore.Lift): void; - onRobotClick?(robot: RomiCore.RobotState): void; + onRobotClick?(fleet: string, robot: RomiCore.RobotState): void; } function calcMaxBounds(mapFloorLayers: readonly MapFloorLayer[]): L.LatLngBounds | undefined { diff --git a/src/components/schedule-visualizer/robots-overlay.tsx b/src/components/schedule-visualizer/robots-overlay.tsx index 01a33f7da..abec1c4f7 100644 --- a/src/components/schedule-visualizer/robots-overlay.tsx +++ b/src/components/schedule-visualizer/robots-overlay.tsx @@ -8,7 +8,7 @@ import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; export interface RobotsOverlayProps extends SVGOverlayProps { colorManager: ColorManager; fleets: readonly RomiCore.FleetState[]; - onRobotClick?(robot: RomiCore.RobotState): void; + onRobotClick?(fleet: string, robot: RomiCore.RobotState): void; conflictRobotNames: string[][]; currentFloorName: string; RobotComponent?: React.ElementType; @@ -51,25 +51,28 @@ export default function RobotsOverlay(props: RobotsOverlayProps): React.ReactEle if (!currentFloorName) { return []; } - return fleets.flatMap(x => x.robots.filter(r => r.location.level_name === currentFloorName)); + return fleets.flatMap(x => ({ + fleet: x.name, + robots: x.robots.filter(r => r.location.level_name === currentFloorName), + })); }, [fleets, currentFloorName]); return ( - {robotsInCurLevel.map(robot => { - return ( + {robotsInCurLevel.map(({ fleet, robots }) => + robots.map(robot => ( onRobotClick && onRobotClick(robot_)} + onClick={(_, robot_) => onRobotClick && onRobotClick(fleet, robot_)} inConflict={inConflict(robot.name)} /> - ); - })} + )), + )} ); From 166f73011d6fc4c3579640013e4e243837aa6bb0 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 14:27:45 +0800 Subject: [PATCH 09/16] optimize commands panel --- src/components/app.tsx | 7 ++++++- src/components/commands-panel.tsx | 24 +++++++++++++++--------- src/components/robots-panel.tsx | 5 +++++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index e04100860..599b7825e 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -137,9 +137,14 @@ export default function App(props: AppProps): JSX.Element { const fleetManager = React.useMemo(() => new FleetManager(), []); const [fleets, setFleets] = React.useState(fleetManager.fleets()); + const fleetNames = React.useRef([]); const [robotSpotlight, setRobotSpotlight] = React.useState | undefined>( undefined, ); + const newFleetNames = fleets.map(fleet => fleet.name); + if (newFleetNames.some(fleetName => !fleetNames.current.includes(fleetName))) { + fleetNames.current = newFleetNames; + } const dispenserStateManager = React.useMemo(() => new DispenserStateManager(), []); const [dispenserStates, setDispenserStates] = React.useState< @@ -362,7 +367,7 @@ export default function App(props: AppProps): JSX.Element { - + diff --git a/src/components/commands-panel.tsx b/src/components/commands-panel.tsx index 531362339..d02353de5 100644 --- a/src/components/commands-panel.tsx +++ b/src/components/commands-panel.tsx @@ -1,16 +1,19 @@ import { - makeStyles, ExpansionPanel, + ExpansionPanelDetails, ExpansionPanelSummary, + makeStyles, Typography, - ExpansionPanelDetails, } from '@material-ui/core'; import { ExpandMore as ExpandMoreIcon } from '@material-ui/icons'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; -import { LoopForm } from './loop-form'; import { v4 as uuidv4 } from 'uuid'; import { RobotDeliveryForm } from './delivery-form'; +import { LoopForm } from './loop-form'; + +const debug = Debug('CommandsPanel'); export type TDeliveryRequest = ( pickupPlaceName: string, @@ -56,7 +59,7 @@ export function requestLoop( } /** -* The Delivery task is one where a robot is assigned to pick up an item at one location (pickup_place_name) and deliver it to another (dropoff_place_name). At each of these locations, there is an automation system called workcell/dispenser that loads and unload the item off the robot. +* The Delivery task is one where a robot is assigned to pick up an item at one location (pickup_place_name) and deliver it to another (dropoff_place_name). At each of these locations, there is an automation system called workcell/dispenser that loads and unload the item off the robot. Currently only these fields are being used in Delivery msg. * task_id: Unique id for the request. @@ -94,14 +97,15 @@ export function requestDelivery( } export interface CommandsPanelProps { - fleets: readonly RomiCore.FleetState[]; + allFleets: string[]; transport?: Readonly; } -export default function CommandsPanel(props: CommandsPanelProps): React.ReactElement { - const { fleets, transport } = props; +export const CommandsPanel = React.memo((props: CommandsPanelProps) => { + debug('render'); + + const { allFleets, transport } = props; const classes = useStyles(); - const allFleets = fleets.flatMap(fleet => fleet.name); const loopRequestPub = React.useMemo( () => (transport ? transport.createPublisher(RomiCore.loopRequests) : null), [transport], @@ -165,7 +169,9 @@ export default function CommandsPanel(props: CommandsPanelProps): React.ReactEle ); -} +}); + +export default CommandsPanel; export const useStyles = makeStyles(theme => ({ expansionSummaryContent: { diff --git a/src/components/robots-panel.tsx b/src/components/robots-panel.tsx index 77a0d147b..c33fff49a 100644 --- a/src/components/robots-panel.tsx +++ b/src/components/robots-panel.tsx @@ -1,8 +1,11 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; import RobotItem, { RobotItemProps } from './robot-item'; import { SpotlightValue } from './spotlight-value'; +const debug = Debug('RobotsPanel'); + export interface RobotsPanelProps { fleets: readonly RomiCore.FleetState[]; spotlight?: Readonly>; @@ -10,6 +13,8 @@ export interface RobotsPanelProps { } export const RobotsPanel = React.memo((props: RobotsPanelProps) => { + debug('render'); + const { fleets, spotlight, onRobotClick } = props; const robotRefs = React.useRef>({}); const [expanded, setExpanded] = React.useState>>({}); From 52b1d736f31b0654a3d76c509f016f49f41920c6 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 15:12:11 +0800 Subject: [PATCH 10/16] memoized schedule visualizer door and lifts rendering --- src/components/app.tsx | 8 +- .../schedule-visualizer/door/door.tsx | 133 ++++++++-------- .../schedule-visualizer/doors-overlay.tsx | 26 ++-- src/components/schedule-visualizer/index.tsx | 14 +- .../schedule-visualizer/lift-overlay.tsx | 23 ++- src/components/schedule-visualizer/lift.tsx | 142 +++++++++--------- src/mock/fake-traj-manager.ts | 5 + 7 files changed, 202 insertions(+), 149 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index 599b7825e..8430bb1ac 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -235,11 +235,11 @@ export default function App(props: AppProps): JSX.Element { setLifts(buildingMap ? buildingMap.lifts : []); }, [buildingMap]); - function handleDoorClick(door: RomiCore.Door): void { + const handleDoorClick = React.useCallback((door: RomiCore.Door) => { setShowOmniPanel(true); setCurrentView(OmniPanelViewIndex.Doors); setDoorSpotlight({ value: door.name }); - } + }, []); function handleRobotClick(fleet: string, robot: RomiCore.RobotState): void { setShowOmniPanel(true); @@ -247,11 +247,11 @@ export default function App(props: AppProps): JSX.Element { setRobotSpotlight({ value: `${fleet}-${robot.name}` }); } - function handleLiftClick(lift: RomiCore.Lift): void { + const handleLiftClick = React.useCallback((lift: RomiCore.Lift) => { setShowOmniPanel(true); setCurrentView(OmniPanelViewIndex.Lifts); setLiftSpotlight({ value: lift.name }); - } + }, []); function clearSpotlights() { setDoorSpotlight(undefined); diff --git a/src/components/schedule-visualizer/door/door.tsx b/src/components/schedule-visualizer/door/door.tsx index fc0d48ede..51870f5c7 100644 --- a/src/components/schedule-visualizer/door/door.tsx +++ b/src/components/schedule-visualizer/door/door.tsx @@ -1,11 +1,14 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; +import React from 'react'; import DefaultDoor from './door-default'; import DoubleHingeDoor from './door-double-hinge'; import DoubleSlideDoor from './door-double-slide'; -import React from 'react'; import SingleHingeDoor from './door-single-hinge'; import SingleSlideDoor from './door-single-slide'; +const debug = Debug('ScheduleVisualizer:Door'); + /** * currentMode: Current mode of the door. E.g: 0 = DoorMode.CLOSE. * door: Door information provided by the map. @@ -38,69 +41,73 @@ export const getDoorStyle = (classes: Record, currentMode: numbe } }; -const Door = React.forwardRef(function( - props: DoorContainerProps, - ref: React.Ref, -): React.ReactElement { - const { door, onClick, currentMode } = props; - const { - DOOR_TYPE_UNDEFINED, - DOOR_TYPE_SINGLE_SLIDING, - DOOR_TYPE_DOUBLE_SLIDING, - DOOR_TYPE_SINGLE_TELESCOPE, - DOOR_TYPE_DOUBLE_TELESCOPE, - DOOR_TYPE_SINGLE_SWING, - DOOR_TYPE_DOUBLE_SWING, - } = RomiCore.Door; - const { door_type: doorType } = door; - const v1 = [door.v1_x, door.v1_y]; - const v2 = [door.v2_x, door.v2_y]; +const Door = React.memo( + React.forwardRef(function( + props: DoorContainerProps, + ref: React.Ref, + ): React.ReactElement { + const { door, onClick, currentMode } = props; + debug('render %s', door.name); + + const { + DOOR_TYPE_UNDEFINED, + DOOR_TYPE_SINGLE_SLIDING, + DOOR_TYPE_DOUBLE_SLIDING, + DOOR_TYPE_SINGLE_TELESCOPE, + DOOR_TYPE_DOUBLE_TELESCOPE, + DOOR_TYPE_SINGLE_SWING, + DOOR_TYPE_DOUBLE_SWING, + } = RomiCore.Door; + const { door_type: doorType } = door; + const v1 = [door.v1_x, door.v1_y]; + const v2 = [door.v2_x, door.v2_y]; - const handleClick = (event: React.MouseEvent) => { - onClick && onClick(event, door); - }; + const handleClick = (event: React.MouseEvent) => { + onClick && onClick(event, door); + }; - return ( - - {doorType === DOOR_TYPE_SINGLE_SWING && ( - - )} - {doorType === DOOR_TYPE_DOUBLE_SWING && ( - - )} - {(doorType === DOOR_TYPE_SINGLE_SLIDING || doorType === DOOR_TYPE_SINGLE_TELESCOPE) && ( - - )} - {(doorType === DOOR_TYPE_DOUBLE_SLIDING || doorType === DOOR_TYPE_DOUBLE_TELESCOPE) && ( - - )} - {doorType === DOOR_TYPE_UNDEFINED && } - - ); -}); + return ( + + {doorType === DOOR_TYPE_SINGLE_SWING && ( + + )} + {doorType === DOOR_TYPE_DOUBLE_SWING && ( + + )} + {(doorType === DOOR_TYPE_SINGLE_SLIDING || doorType === DOOR_TYPE_SINGLE_TELESCOPE) && ( + + )} + {(doorType === DOOR_TYPE_DOUBLE_SLIDING || doorType === DOOR_TYPE_DOUBLE_TELESCOPE) && ( + + )} + {doorType === DOOR_TYPE_UNDEFINED && } + + ); + }), +); export default Door; diff --git a/src/components/schedule-visualizer/doors-overlay.tsx b/src/components/schedule-visualizer/doors-overlay.tsx index 861ba5239..16355a551 100644 --- a/src/components/schedule-visualizer/doors-overlay.tsx +++ b/src/components/schedule-visualizer/doors-overlay.tsx @@ -1,21 +1,21 @@ -/** - * TODO: Need door location, (v1_x, v1_y) only defines the location of the hinge, we also need the - * length and orientation of the door to draw it on the map. - */ - import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React, { useContext } from 'react'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; import { DoorStateContext } from '../rmf-contexts'; -import Door from './door/door'; +import Door, { DoorContainerProps } from './door/door'; import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; +const debug = Debug('ScheduleVisualizer:DoorsOverlay'); + export interface DoorsOverlayProps extends SVGOverlayProps { doors: readonly RomiCore.Door[]; onDoorClick?(door: RomiCore.Door): void; } -export default function DoorsOverlay(props: DoorsOverlayProps): React.ReactElement { +export const DoorsOverlay = React.memo((props: DoorsOverlayProps) => { + debug('render'); + const { doors, onDoorClick, ...otherProps } = props; const viewBox = viewBoxFromLeafletBounds(props.bounds); const doorsState = useContext(DoorStateContext); @@ -24,6 +24,12 @@ export default function DoorsOverlay(props: DoorsOverlayProps): React.ReactEleme const currentDoor = doorsState && doorsState[doorName]; return currentDoor && currentDoor.current_mode.value; }; + + const handleDoorClick = React.useCallback['onClick']>( + (_, door) => onDoorClick && onDoorClick(door), + [onDoorClick], + ); + return ( <> @@ -32,7 +38,7 @@ export default function DoorsOverlay(props: DoorsOverlayProps): React.ReactEleme onDoorClick && onDoorClick(door)} + onClick={handleDoorClick} doorState={doorsState && doorsState[door.name]} currentMode={getCurrentDoorMode(door.name)} /> @@ -41,4 +47,6 @@ export default function DoorsOverlay(props: DoorsOverlayProps): React.ReactEleme ); -} +}); + +export default DoorsOverlay; diff --git a/src/components/schedule-visualizer/index.tsx b/src/components/schedule-visualizer/index.tsx index 9aad0aeba..ebcf889b2 100644 --- a/src/components/schedule-visualizer/index.tsx +++ b/src/components/schedule-visualizer/index.tsx @@ -1,5 +1,6 @@ import { makeStyles } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import * as L from 'leaflet'; import React from 'react'; import { AttributionControl, ImageOverlay, LayersControl, Map as LMap, Pane } from 'react-leaflet'; @@ -27,6 +28,8 @@ import { } from './trajectory-animations'; import WaypointsOverlay from './waypoints-overlay'; +const debug = Debug('ScheduleVisualizer'); + const useStyles = makeStyles(() => ({ map: { height: '100%', @@ -62,6 +65,8 @@ function calcMaxBounds(mapFloorLayers: readonly MapFloorLayer[]): L.LatLngBounds } export default function ScheduleVisualizer(props: ScheduleVisualizerProps): React.ReactElement { + debug('render'); + const { appResources } = props; const classes = useStyles(); const mapRef = React.useRef(null); @@ -178,7 +183,9 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac for (const p of promises) { await p; } + debug('set map floor layers'); setMapFloorLayers(mapFloorLayers); + debug('set max bounds'); setMaxBounds(calcMaxBounds(Object.values(mapFloorLayers))); })(); }, [props.buildingMap, mapElement]); @@ -200,6 +207,7 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac trim: true, }, }); + debug('set trajectories'); setTrajectories(prev => ({ ...prev, [curMapFloorLayer.level.name]: resp, @@ -213,6 +221,7 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac }, [props.trajManager, curMapFloorLayer, trajAnimDuration]); function handleBaseLayerChange(e: L.LayersControlEvent): void { + debug('set current level name'); setCurLevelName(e.name); } @@ -253,11 +262,14 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac if (curMapFloorLayer) { const mapTrajectories = getTrajectory(curMapFloorLayer.level.name); const mapConflicts = getConflicts(curMapFloorLayer.level.name); + debug('set current map trajectories'); setCurMapTrajectories(mapTrajectories); + debug('set current map conflicts'); setCurMapConflicts(mapConflicts); + debug('set conflicting robots'); setConflictRobotNames(getConflictRobotsName(mapConflicts, mapTrajectories)); } - }, [curMapFloorLayer, trajectories, curMapConflicts, curMapTrajectories]); + }, [curMapFloorLayer, trajectories]); return ( { + debug('render'); + const { lifts, onLiftClick, currentFloor, ...otherProps } = props; const viewBox = viewBoxFromLeafletBounds(props.bounds); const liftsState = useContext(LiftStateContext); + + const handleLiftClick = React.useCallback['onClick']>( + (_, lift) => onLiftClick && onLiftClick(lift), + [onLiftClick], + ); + return ( <> {lifts.map(lift => ( onLiftClick && onLiftClick(lift)} + onClick={handleLiftClick} liftState={liftsState && liftsState[lift.name]} currentFloor={currentFloor} /> @@ -33,4 +44,6 @@ export default function LiftsOverlay(props: LiftsOverlayProps): React.ReactEleme ); -} +}); + +export default LiftsOverlay; diff --git a/src/components/schedule-visualizer/lift.tsx b/src/components/schedule-visualizer/lift.tsx index da127c156..15b1b2d08 100644 --- a/src/components/schedule-visualizer/lift.tsx +++ b/src/components/schedule-visualizer/lift.tsx @@ -1,12 +1,15 @@ import { makeStyles } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; -import Door from './door/door'; -import { UpArrow, DownArrow } from './arrow'; import { radiansToDegrees, transformMiddleCoordsOfRectToSVGBeginPoint, } from '../../util/calculation-helpers'; +import { DownArrow, UpArrow } from './arrow'; +import Door from './door/door'; + +const debug = Debug('ScheduleVisualizer:Lift'); export interface LiftProps { id?: string; @@ -72,75 +75,80 @@ const getRelatedYCoord = (topY: number, height: number, percentage: number) => { return topY + height * percentage; }; -const Lift = React.forwardRef(function( - props: LiftProps, - ref: React.Ref, -): React.ReactElement { - const { id, lift, onClick, liftState, currentFloor } = props; - const { width, depth, ref_x: x, ref_y: y, ref_yaw, doors } = lift; - // The svg start drawing from the top left coordinate, and the backend sends us the middle X,Y so we need to transform that. - const { x: topVerticeX, y: topVerticeY } = transformMiddleCoordsOfRectToSVGBeginPoint( - x, - y, - width, - depth, - ); - // Since we are working with a plane with positive X and Negative Y we need to change the sign of the y. - const contextY = -y; - const contextTopVerticeY = -topVerticeY; - // Get properties from lift state - const currentMode = liftState?.current_mode; - const destinationFloor = liftState?.destination_floor; - const doorState = liftState?.door_state; - const isInCurrentFloor = liftState?.current_floor === currentFloor; - const motionState = liftState?.motion_state; +const Lift = React.memo( + React.forwardRef(function(props: LiftProps, ref: React.Ref): React.ReactElement { + const { id, lift, onClick, liftState, currentFloor } = props; + debug('render %s', lift.name); + + const { width, depth, ref_x: x, ref_y: y, ref_yaw, doors } = lift; + // The svg start drawing from the top left coordinate, and the backend sends us the middle X,Y so we need to transform that. + const { x: topVerticeX, y: topVerticeY } = transformMiddleCoordsOfRectToSVGBeginPoint( + x, + y, + width, + depth, + ); + // Since we are working with a plane with positive X and Negative Y we need to change the sign of the y. + const contextY = -y; + const contextTopVerticeY = -topVerticeY; + // Get properties from lift state + const currentMode = liftState?.current_mode; + const destinationFloor = liftState?.destination_floor; + const doorState = liftState?.door_state; + const isInCurrentFloor = liftState?.current_floor === currentFloor; + const motionState = liftState?.motion_state; - const classes = useStyles(); + const classes = useStyles(); - const liftStyle = getLiftStyle(classes, currentMode, isInCurrentFloor); - const liftMotionText = getLiftMotionText(liftState?.current_floor, destinationFloor, motionState); - const liftModeText = getLiftModeText(currentMode); - return ( - <> - onClick && onClick(e, lift)}> - - {liftMotionText && ( - - {liftMotionText} - + const liftStyle = getLiftStyle(classes, currentMode, isInCurrentFloor); + const liftMotionText = getLiftMotionText( + liftState?.current_floor, + destinationFloor, + motionState, + ); + const liftModeText = getLiftModeText(currentMode); + return ( + <> + onClick && onClick(e, lift)}> + + {liftMotionText && ( + + {liftMotionText} + + )} + {liftModeText && ( + + {liftModeText} + + )} + + {motionState === RomiCore.LiftState.MOTION_UP && ( + )} - {liftModeText && ( - - {liftModeText} - + {motionState === RomiCore.LiftState.MOTION_DOWN && ( + )} - - {motionState === RomiCore.LiftState.MOTION_UP && ( - - )} - {motionState === RomiCore.LiftState.MOTION_DOWN && ( - - )} - {doors.map(door => ( - - ))} - - ); -}); + {doors.map(door => ( + + ))} + + ); + }), +); export default Lift; diff --git a/src/mock/fake-traj-manager.ts b/src/mock/fake-traj-manager.ts index c4fdd266b..d83b3ff88 100644 --- a/src/mock/fake-traj-manager.ts +++ b/src/mock/fake-traj-manager.ts @@ -1,3 +1,4 @@ +import Debug from 'debug'; import { RobotTrajectoryManager, TimeRequest, @@ -7,14 +8,18 @@ import { } from '../robot-trajectory-manager'; import trajectories from './data/trajectories.json'; +const debug = Debug('FakeTrajectoryManager'); + export default class FakeTrajectoryManager implements RobotTrajectoryManager { async latestTrajectory(request: TrajectoryRequest): Promise { + debug('sending trajectory'); if (request.param.map_name === 'L1') { const traj = trajectories[this.currentTraj++]; this.currentTraj %= trajectories.length; // "deep clone" object return JSON.parse(JSON.stringify(traj)) as any; } + return { response: 'trajectory', values: [], From ce73e9fa7a516ebc6d546403fceb44c85ee492cf Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 15:21:26 +0800 Subject: [PATCH 11/16] memoized robots overlay --- src/components/app.tsx | 4 +- src/components/schedule-visualizer/robot.tsx | 108 +++++++++--------- .../schedule-visualizer/robots-overlay.tsx | 18 ++- 3 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index 8430bb1ac..3195b57f3 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -241,11 +241,11 @@ export default function App(props: AppProps): JSX.Element { setDoorSpotlight({ value: door.name }); }, []); - function handleRobotClick(fleet: string, robot: RomiCore.RobotState): void { + const handleRobotClick = React.useCallback((fleet: string, robot: RomiCore.RobotState) => { setShowOmniPanel(true); setCurrentView(OmniPanelViewIndex.Robots); setRobotSpotlight({ value: `${fleet}-${robot.name}` }); - } + }, []); const handleLiftClick = React.useCallback((lift: RomiCore.Lift) => { setShowOmniPanel(true); diff --git a/src/components/schedule-visualizer/robot.tsx b/src/components/schedule-visualizer/robot.tsx index 545e0c9c7..973dc2337 100644 --- a/src/components/schedule-visualizer/robot.tsx +++ b/src/components/schedule-visualizer/robot.tsx @@ -1,13 +1,16 @@ import { makeStyles } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React, { useContext, useState } from 'react'; -import { ResourcesContext } from '../app-contexts'; import ResourceManager from '../../resource-manager'; +import { ResourcesContext } from '../app-contexts'; import ColorManager from './colors'; import RobotDefaultIcon from './robot-default-icon'; import RobotImageIcon from './robot-image-icon'; import SvgText from './svg-text'; +const debug = Debug('ScheduleVisualizer:Robot'); + const useStyles = makeStyles(() => ({ robotText: { dominantBaseline: 'central', @@ -32,60 +35,61 @@ export interface RobotProps { footprint: number; fleetName: string; inConflict?: boolean; - onClick?(e: React.MouseEvent, robot: RomiCore.RobotState): void; + onClick?(e: React.MouseEvent, fleetName: string, robot: RomiCore.RobotState): void; } -const Robot = React.forwardRef(function( - props: RobotProps, - ref: React.Ref, -): React.ReactElement { - const resourcesContext = useContext(ResourcesContext); - const classes = useStyles(); - const { robot, footprint, colorManager, fleetName, inConflict, onClick } = props; - // The only image formats SVG software support are JPEG, PNG, and other SVG files. - const [renderCustomIcon, setRenderCustomIcon] = useState({ - path: ResourceManager.getRobotIconPath(resourcesContext, fleetName), - error: false, - }); +const Robot = React.memo( + React.forwardRef(function(props: RobotProps, ref: React.Ref): React.ReactElement { + const resourcesContext = useContext(ResourcesContext); + const classes = useStyles(); + const { robot, footprint, colorManager, fleetName, inConflict, onClick } = props; + debug('render %s', robot.name); + + // The only image formats SVG software support are JPEG, PNG, and other SVG files. + const [renderCustomIcon, setRenderCustomIcon] = useState({ + path: ResourceManager.getRobotIconPath(resourcesContext, fleetName), + error: false, + }); - return ( - <> - onClick && onClick(e, robot)} - transform={`translate(${robot.location.x} ${-robot.location.y}) + return ( + <> + onClick && onClick(e, fleetName, robot)} + transform={`translate(${robot.location.x} ${-robot.location.y}) rotate(${-(robot.location.yaw * 180) / Math.PI})`} - > - {!!renderCustomIcon.path && !renderCustomIcon.error ? ( - - ) : ( - - )} - - - - ); -}); + > + {!!renderCustomIcon.path && !renderCustomIcon.error ? ( + + ) : ( + + )} + + + + ); + }), +); export default Robot; diff --git a/src/components/schedule-visualizer/robots-overlay.tsx b/src/components/schedule-visualizer/robots-overlay.tsx index abec1c4f7..ed393283c 100644 --- a/src/components/schedule-visualizer/robots-overlay.tsx +++ b/src/components/schedule-visualizer/robots-overlay.tsx @@ -1,10 +1,13 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React, { useMemo } from 'react'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; import ColorManager from './colors'; import Robot_, { RobotProps } from './robot'; import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; +const debug = Debug('ScheduleVisualizer:RobotsOverlay'); + export interface RobotsOverlayProps extends SVGOverlayProps { colorManager: ColorManager; fleets: readonly RomiCore.FleetState[]; @@ -14,7 +17,9 @@ export interface RobotsOverlayProps extends SVGOverlayProps { RobotComponent?: React.ElementType; } -export default function RobotsOverlay(props: RobotsOverlayProps): React.ReactElement { +export const RobotsOverlay = React.memo((props: RobotsOverlayProps) => { + debug('render'); + const { fleets, colorManager, @@ -57,6 +62,11 @@ export default function RobotsOverlay(props: RobotsOverlayProps): React.ReactEle })); }, [fleets, currentFloorName]); + const handleRobotClick = React.useCallback['onClick']>( + (_, fleetName, robot) => onRobotClick && onRobotClick(fleetName, robot), + [onRobotClick], + ); + return ( @@ -68,7 +78,7 @@ export default function RobotsOverlay(props: RobotsOverlayProps): React.ReactEle fleetName={fleetContainer[`${robot.name}_${robot.model}`]} footprint={footprint} colorManager={colorManager} - onClick={(_, robot_) => onRobotClick && onRobotClick(fleet, robot_)} + onClick={handleRobotClick} inConflict={inConflict(robot.name)} /> )), @@ -76,4 +86,6 @@ export default function RobotsOverlay(props: RobotsOverlayProps): React.ReactEle ); -} +}); + +export default RobotsOverlay; From d8975d71e645fc33724edbb8183fe3202be92292 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 15:36:26 +0800 Subject: [PATCH 12/16] memoized robot trajectories --- src/components/schedule-visualizer/index.tsx | 13 ++-- .../robot-trajectories-overlay.tsx | 15 ++-- .../schedule-visualizer/robot-trajectory.tsx | 77 ++++++++++--------- 3 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/components/schedule-visualizer/index.tsx b/src/components/schedule-visualizer/index.tsx index ebcf889b2..a41da7d3d 100644 --- a/src/components/schedule-visualizer/index.tsx +++ b/src/components/schedule-visualizer/index.tsx @@ -115,17 +115,18 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac return 1000; } }, [settings]); - const TrajectoryComponent = React.useMemo(() => { + + const RobotTrajContextValue = React.useMemo(() => { const animationScale = trajLookahead / trajAnimDuration; switch (settings.trajectoryAnimation) { case TrajectoryAnimation.None: - return RobotTrajectory; + return { Component: RobotTrajectory }; case TrajectoryAnimation.Fill: - return withFillAnimation(RobotTrajectory, animationScale); + return { Component: withFillAnimation(RobotTrajectory, animationScale) }; case TrajectoryAnimation.Follow: - return withFollowAnimation(RobotTrajectory, animationScale); + return { Component: withFollowAnimation(RobotTrajectory, animationScale) }; case TrajectoryAnimation.Outline: - return withOutlineAnimation(RobotTrajectory, animationScale); + return { Component: withOutlineAnimation(RobotTrajectory, animationScale) }; } }, [settings.trajectoryAnimation, trajAnimDuration]); @@ -303,7 +304,7 @@ export default function ScheduleVisualizer(props: ScheduleVisualizerProps): Reac {curMapFloorLayer && ( - + { + debug('render'); + const { trajs, conflicts, colorManager, conflictRobotNames, ...otherProps } = props; const trajectoryContext = useContext(RobotTrajectoryContext); const notificationDispatch = useContext(NotificationBarContext); @@ -61,7 +64,9 @@ export default function RobotTrajectoriesOverlay( ); -} +}); + +export default RobotTrajectoriesOverlay; export interface RobotTrajectoryContext { Component: React.ComponentType; diff --git a/src/components/schedule-visualizer/robot-trajectory.tsx b/src/components/schedule-visualizer/robot-trajectory.tsx index 7584ded6a..efd722055 100644 --- a/src/components/schedule-visualizer/robot-trajectory.tsx +++ b/src/components/schedule-visualizer/robot-trajectory.tsx @@ -1,8 +1,11 @@ +import { useTheme } from '@material-ui/core'; +import Debug from 'debug'; import React from 'react'; -import { Conflict, rawKnotsToKnots, Trajectory, RawKnot } from '../../robot-trajectory-manager'; +import { Conflict, RawKnot, rawKnotsToKnots, Trajectory } from '../../robot-trajectory-manager'; import { bezierControlPoints, knotsToSegmentCoefficientsArray } from '../../util/cublic-spline'; import { TrajectoryPath } from './trajectory-animations'; -import { useTheme } from '@material-ui/core'; + +const debug = Debug('ScheduleVisualizer:RobotTrajectory'); export interface RobotTrajectoryProps extends React.RefAttributes, @@ -12,42 +15,46 @@ export interface RobotTrajectoryProps footprint: number; } -export const RobotTrajectory = React.forwardRef(function( - props: RobotTrajectoryProps, - ref: React.Ref, -): React.ReactElement { - const { trajectory, conflicts, footprint, ...otherProps } = props; - const theme = useTheme(); +export const RobotTrajectory = React.memo( + React.forwardRef(function( + props: RobotTrajectoryProps, + ref: React.Ref, + ): React.ReactElement { + debug('render'); - const color = React.useMemo( - () => - conflicts.flat().includes(trajectory.id) - ? theme.palette.error.main - : theme.palette.success.main, - [trajectory, conflicts, theme], - ); + const { trajectory, conflicts, footprint, ...otherProps } = props; + const theme = useTheme(); - const pathD = React.useMemo(() => { - return trajectoryPath(trajectory.segments).d; - }, [trajectory]); + const color = React.useMemo( + () => + conflicts.flat().includes(trajectory.id) + ? theme.palette.error.main + : theme.palette.success.main, + [trajectory, conflicts, theme], + ); - return ( - - ); -}); + const pathD = React.useMemo(() => { + return trajectoryPath(trajectory.segments).d; + }, [trajectory]); + + return ( + + ); + }), +); export default RobotTrajectory; From 1d9ad04f846381f475119706cf3e38ca03666bd2 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 15:41:31 +0800 Subject: [PATCH 13/16] memoized waypoints --- src/components/schedule-visualizer/waypoint.tsx | 9 +++++++-- .../schedule-visualizer/waypoints-overlay.tsx | 11 +++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/components/schedule-visualizer/waypoint.tsx b/src/components/schedule-visualizer/waypoint.tsx index 695a66b76..dfceae223 100644 --- a/src/components/schedule-visualizer/waypoint.tsx +++ b/src/components/schedule-visualizer/waypoint.tsx @@ -1,14 +1,19 @@ import { makeStyles } from '@material-ui/core'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React from 'react'; +const debug = Debug('ScheduleVisualizer:Waypoint'); + export interface WaypointProps { waypoint: RomiCore.GraphNode; size: number; } -const Waypoint = (props: WaypointProps): React.ReactElement => { +const Waypoint = React.memo((props: WaypointProps) => { const { waypoint, size } = props; + debug('render %s', waypoint.name); + const classes = useStyles(); return ( @@ -34,7 +39,7 @@ const Waypoint = (props: WaypointProps): React.ReactElement => { ); -}; +}); export default Waypoint; diff --git a/src/components/schedule-visualizer/waypoints-overlay.tsx b/src/components/schedule-visualizer/waypoints-overlay.tsx index 29dab285f..ccc39af90 100644 --- a/src/components/schedule-visualizer/waypoints-overlay.tsx +++ b/src/components/schedule-visualizer/waypoints-overlay.tsx @@ -1,14 +1,19 @@ import * as RomiCore from '@osrf/romi-js-core-interfaces'; +import Debug from 'debug'; import React, { useMemo } from 'react'; import { viewBoxFromLeafletBounds } from '../../util/css-utils'; import SVGOverlay, { SVGOverlayProps } from './svg-overlay'; import Waypoint from './waypoint'; +const debug = Debug('ScheduleVisualier:WaypointsOverlay'); + export interface WaypointsOverlayProps extends SVGOverlayProps { currentLevel: RomiCore.Level; } -export default function WaypointsOverlay(props: WaypointsOverlayProps): React.ReactElement { +export const WaypointsOverlay = React.memo((props: WaypointsOverlayProps) => { + debug('render'); + const { currentLevel, ...otherProps } = props; const viewBox = viewBoxFromLeafletBounds(props.bounds); // Set the size of the waypoint. At least for now we don't want for this to change. We left this here in case we want for this to change in the future. @@ -30,4 +35,6 @@ export default function WaypointsOverlay(props: WaypointsOverlayProps): React.Re ); -} +}); + +export default WaypointsOverlay; From 166f77e46d2bb02559eb437268dec77f975b6849 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 15:44:56 +0800 Subject: [PATCH 14/16] fix tests --- .../tests/__snapshots__/waypoint.test.tsx.snap | 4 ++-- src/components/tests/commands-panel.test.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/schedule-visualizer/tests/__snapshots__/waypoint.test.tsx.snap b/src/components/schedule-visualizer/tests/__snapshots__/waypoint.test.tsx.snap index 1fbd983c7..b6ef63416 100644 --- a/src/components/schedule-visualizer/tests/__snapshots__/waypoint.test.tsx.snap +++ b/src/components/schedule-visualizer/tests/__snapshots__/waypoint.test.tsx.snap @@ -2,7 +2,7 @@ exports[`Renders correctly 1`] = ` - - + `; diff --git a/src/components/tests/commands-panel.test.tsx b/src/components/tests/commands-panel.test.tsx index 5592bdf74..98564b7c4 100644 --- a/src/components/tests/commands-panel.test.tsx +++ b/src/components/tests/commands-panel.test.tsx @@ -14,7 +14,7 @@ beforeEach(() => { }); it('renders loop form', () => { - const root = mount(); + const root = mount( fleet.name)} />); const formElements = root.find(LoopForm); expect(formElements.length).toBe(1); root.unmount(); From b8a278e1527c37a6904acfb55e156bcd9e586f35 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 16:49:17 +0800 Subject: [PATCH 15/16] memoized main menu --- src/components/app.tsx | 54 +++++++++++++++++++---------------- src/components/main-menu.tsx | 11 +++++-- src/components/omni-panel.tsx | 51 +++++++++++++++++++++++---------- 3 files changed, 74 insertions(+), 42 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index 3195b57f3..9523a4a55 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -260,40 +260,47 @@ export default function App(props: AppProps): JSX.Element { setDispenserSpotlight(undefined); } - function handleClose() { + const handleClose = React.useCallback(() => { clearSpotlights(); setShowOmniPanel(false); - } + }, []); - function handleBack(index: number): void { - clearSpotlights(); - const parent = viewMap[index].parent; - if (!parent) { - return handleClose(); - } - setCurrentView(parent.value); - } + const handleBack = React.useCallback( + (index: number) => { + clearSpotlights(); + const parent = viewMap[index].parent; + if (!parent) { + return handleClose(); + } + setCurrentView(parent.value); + }, + [handleClose], + ); - function handleMainMenuDoorsClick(): void { + const handleMainMenuDoorsClick = React.useCallback(() => { setCurrentView(OmniPanelViewIndex.Doors); - } + }, []); - function handleMainMenuLiftsClick(): void { - setLiftStates(liftStateManager.liftStates()); + const handleMainMenuLiftsClick = React.useCallback(() => { setCurrentView(OmniPanelViewIndex.Lifts); - } + }, []); - function handleMainMenuRobotsClick(): void { + const handleMainMenuRobotsClick = React.useCallback(() => { setCurrentView(OmniPanelViewIndex.Robots); - } + }, []); - function handleMainMenuDispensersClick(): void { + const handleMainMenuDispensersClick = React.useCallback(() => { setCurrentView(OmniPanelViewIndex.Dispensers); - } + }, []); - function handleMainMenuCommandsClick(): void { + const handleMainMenuCommandsClick = React.useCallback(() => { setCurrentView(OmniPanelViewIndex.Commands); - } + }, []); + + const omniPanelClasses = React.useMemo( + () => ({ backButton: classes.topLeftBorder, closeButton: classes.topRightBorder }), + [classes.topLeftBorder, classes.topRightBorder], + ); return ( @@ -327,10 +334,7 @@ export default function App(props: AppProps): JSX.Element { ): void; @@ -9,7 +12,9 @@ export interface MainMenuProps { onCommandsClick?(event: React.MouseEvent): void; } -export default function MainMenu(props: MainMenuProps): React.ReactElement { +export const MainMenu = React.memo((props: MainMenuProps) => { + debug('render'); + return ( @@ -41,4 +46,6 @@ export default function MainMenu(props: MainMenuProps): React.ReactElement { ); -} \ No newline at end of file +}); + +export default MainMenu; diff --git a/src/components/omni-panel.tsx b/src/components/omni-panel.tsx index 33573d7a6..f67154aa7 100644 --- a/src/components/omni-panel.tsx +++ b/src/components/omni-panel.tsx @@ -2,6 +2,7 @@ import { Button, ButtonGroup, makeStyles, Slide } from '@material-ui/core'; import { Close as CloseIcon, KeyboardBackspace as BackIcon } from '@material-ui/icons'; import * as RomiCore from '@osrf/romi-js-core-interfaces'; import React from 'react'; +import { isArray } from 'util'; import { OmniPanelViewProps } from './omni-panel-view'; const useStyles = makeStyles(() => ({ @@ -33,7 +34,7 @@ export interface OmniPanelProps extends React.HTMLAttributes { }; onBack?: (current: number) => void; onClose?: () => void; - children?: React.ReactElement[]; + children?: React.ReactElement[] | React.ReactElement; } export const OmniPanel = React.forwardRef(function( @@ -54,6 +55,39 @@ export const OmniPanel = React.forwardRef(function( props.onClose && props.onClose(); } + const renderChildren = () => { + if (!children) { + return null; + } + if (isArray(children)) { + return children.map(child => ( + +
{child}
+ + )); + } else { + return ( + +
{children}
+ + ); + } + }; + return (
@@ -74,20 +108,7 @@ export const OmniPanel = React.forwardRef(function( -
- {props.children?.map(child => ( - -
{child}
- - ))} -
+
{renderChildren()}
); }); From 5624cc350253eafc7899051b4ce29c463e1ae278 Mon Sep 17 00:00:00 2001 From: koonpeng Date: Thu, 3 Sep 2020 16:52:10 +0800 Subject: [PATCH 16/16] debug(): add OmniPanel prefix to omni panel components --- src/components/commands-panel.tsx | 2 +- src/components/dispenser-item.tsx | 2 +- src/components/dispensers-panel.tsx | 2 +- src/components/door-item.tsx | 2 +- src/components/doors-panel.tsx | 2 +- src/components/lift-item/lift-item.tsx | 2 +- src/components/lift-item/lifts-panel.tsx | 2 +- src/components/robot-item.tsx | 2 +- src/components/robots-panel.tsx | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/commands-panel.tsx b/src/components/commands-panel.tsx index d02353de5..06280f4d1 100644 --- a/src/components/commands-panel.tsx +++ b/src/components/commands-panel.tsx @@ -13,7 +13,7 @@ import { v4 as uuidv4 } from 'uuid'; import { RobotDeliveryForm } from './delivery-form'; import { LoopForm } from './loop-form'; -const debug = Debug('CommandsPanel'); +const debug = Debug('OmniPanel:CommandsPanel'); export type TDeliveryRequest = ( pickupPlaceName: string, diff --git a/src/components/dispenser-item.tsx b/src/components/dispenser-item.tsx index ee144a22b..e47f2651f 100644 --- a/src/components/dispenser-item.tsx +++ b/src/components/dispenser-item.tsx @@ -18,7 +18,7 @@ import { colorPalette } from '../util/css-utils'; import DisableableTypography from './disableable-typography'; import OmniPanelStatusLabels from './omni-panel-status-labels'; -const debug = Debug('DispenserItem'); +const debug = Debug('OmniPanel:DispenserItem'); export interface DispenserItemProps extends Omit { dispenserState: Readonly; diff --git a/src/components/dispensers-panel.tsx b/src/components/dispensers-panel.tsx index fa218e2de..01f6eac14 100644 --- a/src/components/dispensers-panel.tsx +++ b/src/components/dispensers-panel.tsx @@ -4,7 +4,7 @@ import React from 'react'; import DispenserItem, { DispenserItemProps } from './dispenser-item'; import { SpotlightValue } from './spotlight-value'; -const debug = Debug('DispenserPanel'); +const debug = Debug('OmniPanel:DispenserPanel'); export interface DispenserPanelProps { dispenserStates: Readonly>; diff --git a/src/components/door-item.tsx b/src/components/door-item.tsx index 29aedd7b0..fd9df1fcc 100644 --- a/src/components/door-item.tsx +++ b/src/components/door-item.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { colorPalette } from '../util/css-utils'; import OmniPanelStatusLabels from './omni-panel-status-labels'; -const debug = Debug('DoorItem'); +const debug = Debug('OmniPanel:DoorItem'); export interface DoorItemProps extends Omit { door: Readonly; diff --git a/src/components/doors-panel.tsx b/src/components/doors-panel.tsx index 5b4eb386d..fbf826e27 100644 --- a/src/components/doors-panel.tsx +++ b/src/components/doors-panel.tsx @@ -5,7 +5,7 @@ import { makeCallbackArrayCallback } from '../util/react-helpers'; import DoorItem_, { DoorItemProps } from './door-item'; import { SpotlightValue } from './spotlight-value'; -const debug = Debug('DoorsPanel'); +const debug = Debug('OmniPanel:DoorsPanel'); const DoorItem = React.memo(DoorItem_); export interface DoorsPanelProps { diff --git a/src/components/lift-item/lift-item.tsx b/src/components/lift-item/lift-item.tsx index 75aca4f9e..cf2024165 100644 --- a/src/components/lift-item/lift-item.tsx +++ b/src/components/lift-item/lift-item.tsx @@ -16,7 +16,7 @@ import OmniPanelStatusLabels from '../omni-panel-status-labels'; import { colorPalette } from '../../util/css-utils'; import Debug from 'debug'; -const debug = Debug('LiftItem'); +const debug = Debug('OmniPanel:LiftItem'); export interface LiftItemProps extends Omit { id?: string; diff --git a/src/components/lift-item/lifts-panel.tsx b/src/components/lift-item/lifts-panel.tsx index 653efc39d..b4a07bd0f 100644 --- a/src/components/lift-item/lifts-panel.tsx +++ b/src/components/lift-item/lifts-panel.tsx @@ -6,7 +6,7 @@ import { makeCallbackArrayCallback } from '../../util/react-helpers'; import { SpotlightValue } from '../spotlight-value'; import { LiftItem, LiftItemProps } from './lift-item'; -const debug = Debug('LiftsPanel'); +const debug = Debug('OmniPanel:LiftsPanel'); export interface LiftsPanelProps { lifts: RomiCore.Lift[]; diff --git a/src/components/robot-item.tsx b/src/components/robot-item.tsx index f5d13155f..39d44318e 100644 --- a/src/components/robot-item.tsx +++ b/src/components/robot-item.tsx @@ -12,7 +12,7 @@ import React from 'react'; import OmniPanelStatusLabels from './omni-panel-status-labels'; import { RobotInformation } from './robot-item-information'; -const debug = Debug('RobotItem'); +const debug = Debug('OmniPanel:RobotItem'); export interface RobotItemProps extends Omit { fleetName: string; diff --git a/src/components/robots-panel.tsx b/src/components/robots-panel.tsx index c33fff49a..afaf25604 100644 --- a/src/components/robots-panel.tsx +++ b/src/components/robots-panel.tsx @@ -4,7 +4,7 @@ import React from 'react'; import RobotItem, { RobotItemProps } from './robot-item'; import { SpotlightValue } from './spotlight-value'; -const debug = Debug('RobotsPanel'); +const debug = Debug('OmniPanel:RobotsPanel'); export interface RobotsPanelProps { fleets: readonly RomiCore.FleetState[];