From 24d446e47b0d72c172aa78d3f0227646b0260786 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Mon, 15 Apr 2019 08:12:52 -0700 Subject: [PATCH] Extract focus management to a component --- js/components/App.tsx | 81 +----------- js/components/EqualizerWindow/index.tsx | 73 +++++------ js/components/FocusTarget.tsx | 66 ++++++++++ js/components/GenWindow/index.tsx | 93 ++++++------- js/components/MainWindow/index.tsx | 121 +++++++++-------- .../MilkdropWindow/PresetOverlay.tsx | 14 +- js/components/MilkdropWindow/index.tsx | 49 ++++--- js/components/PlaylistWindow/index.tsx | 123 +++++++++--------- 8 files changed, 297 insertions(+), 323 deletions(-) create mode 100644 js/components/FocusTarget.tsx diff --git a/js/components/App.tsx b/js/components/App.tsx index 6ca70a17..d6519175 100644 --- a/js/components/App.tsx +++ b/js/components/App.tsx @@ -55,18 +55,9 @@ type Props = StateProps & DispatchProps & OwnProps; * Constructs the windows to render, and tracks focus. */ class App extends React.Component { - _emitter: Emitter; - _bindings: { - [windowId: string]: { node: HTMLElement; remove(): void } | null; - }; - _webampNode: HTMLDivElement | null; constructor(props: Props) { super(props); - // TODO #leak - this._emitter = new Emitter(); - // TODO #leak - this._bindings = {}; this._webampNode = null; } @@ -94,16 +85,6 @@ class App extends React.Component { } } - componentDidMount() { - this._setFocus(); - } - - componentDidUpdate(prevProps: Props) { - if (prevProps.focused !== this.props.focused) { - this._setFocus(); - } - } - _handleWindowResize = () => { if (this._webampNode == null) { return; @@ -125,41 +106,6 @@ class App extends React.Component { this._webampNode.style.overflow = "visible"; }; - _setFocus() { - const binding = this._bindings[this.props.focused]; - if (binding && binding.node) { - binding.node.focus(); - } - } - - _gotRef(windowId: WindowId, comp: React.Component | null) { - if (comp == null) { - const binding = this._bindings[windowId]; - if (binding != null && binding.remove) { - binding.remove(); - } - this._bindings[windowId] = null; - return; - } - - const node = ReactDOM.findDOMNode(comp) as HTMLElement; - const binding = this._bindings[windowId]; - if (node == null || (binding && binding.node === node)) { - return; - } - - node.tabIndex = -1; - const listener = (e: Event) => this._emitter.trigger(windowId, e); - node.addEventListener("keydown", listener); - - this._bindings[windowId] = { - node, - remove: () => { - node.removeEventListener("keydown", listener); - }, - }; - } - _renderWindows() { const { media, genWindowsInfo, filePickers } = this.props; return Utils.objectMap(genWindowsInfo, (w, id) => { @@ -170,37 +116,18 @@ class App extends React.Component { case WINDOWS.MAIN: return ( this._gotRef(id, component)} analyser={media.getAnalyser()} filePickers={filePickers} /> ); case WINDOWS.EQUALIZER: - return ( - this._gotRef(id, component)} /> - ); + return ; case WINDOWS.PLAYLIST: - return ( - this._gotRef(id, component)} - analyser={media.getAnalyser()} - /> - ); + return ; case WINDOWS.MEDIA_LIBRARY: - return ( - this._gotRef(id, component)} - /> - ); + return ; case WINDOWS.MILKDROP: - return ( - this._gotRef(id, component)} - // TODO: Refactor this. I don't think we need this to be generic anymore. - onFocusedKeyDown={listener => this._emitter.on(id, listener)} - analyser={media.getAnalyser()} - /> - ); + return ; default: throw new Error(`Tried to render an unknown window: ${id}`); } diff --git a/js/components/EqualizerWindow/index.tsx b/js/components/EqualizerWindow/index.tsx index 0fb641a3..8cfce998 100644 --- a/js/components/EqualizerWindow/index.tsx +++ b/js/components/EqualizerWindow/index.tsx @@ -13,7 +13,6 @@ import { toggleEqualizerShadeMode, } from "../../actionCreators"; -import { SET_FOCUSED_WINDOW } from "../../actionTypes"; import { getWindowShade } from "../../selectors"; import Band from "./Band"; @@ -25,6 +24,7 @@ import EqualizerShade from "./EqualizerShade"; import "../../../css/equalizer-window.css"; import { Dispatch, Band as BandType, AppState } from "../../types"; +import FocusTarget from "../FocusTarget"; interface StateProps { doubled: boolean; @@ -33,7 +33,6 @@ interface StateProps { } interface DispatchProps { - focusWindow(): void; setPreampValue(value: number): void; setEqToMin(): void; setEqToMid(): void; @@ -56,43 +55,41 @@ const EqualizerWindow = (props: StateProps & DispatchProps) => { draggable: true, }); return ( -
- {props.shade ? ( - - ) : ( -
-
+
+ + {props.shade ? ( + + ) : ( +
-
+ className="equalizer-top title-bar draggable" + onDoubleClick={props.toggleEqualizerShadeMode} + > +
+
+
+ + + + + +
+
+
+ {BANDS.map(hertz => ( + + ))}
- - - - - -
-
-
- {BANDS.map(hertz => ( - - ))} -
- )} + )} +
); }; @@ -100,8 +97,6 @@ const EqualizerWindow = (props: StateProps & DispatchProps) => { // This does not use the shorthand object syntax becuase `setHertzValue` needs // to return a function. const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => ({ - focusWindow: () => - dispatch({ type: SET_FOCUSED_WINDOW, window: WINDOWS.EQUALIZER }), setPreampValue: (value: number) => dispatch(setPreamp(value)), setEqToMin: () => dispatch(setEqToMin()), setEqToMid: () => dispatch(setEqToMid()), diff --git a/js/components/FocusTarget.tsx b/js/components/FocusTarget.tsx new file mode 100644 index 00000000..655c6def --- /dev/null +++ b/js/components/FocusTarget.tsx @@ -0,0 +1,66 @@ +import React, { useCallback } from "react"; +import { WindowId, AppState, Dispatch } from "../types"; +import * as Actions from "../actionCreators"; +import * as Selectors from "../selectors"; +import { connect } from "react-redux"; + +interface DispatchProps { + setFocus(windowId: WindowId): void; +} +interface StateProps { + focusedWindowId: WindowId; +} + +interface OwnProps { + onKeyDown?(e: React.KeyboardEvent): void; + windowId: WindowId; + children: React.ReactNode; +} + +type Props = StateProps & DispatchProps & OwnProps; + +function FocusTarget(props: Props) { + const { onKeyDown, focusedWindowId, windowId, setFocus, children } = props; + const keyDownHandler = useCallback( + e => { + if (windowId === focusedWindowId && onKeyDown != null) { + onKeyDown(e); + } + }, + [onKeyDown, windowId, focusedWindowId] + ); + + const focusHandler = useCallback(() => { + if (windowId !== focusedWindowId) { + setFocus(windowId); + } + }, [windowId, focusedWindowId]); + + return ( +
+ {children} +
+ ); +} + +function mapStateToProps(state: AppState): StateProps { + return { + focusedWindowId: Selectors.getFocusedWindow(state), + }; +} + +function mapDispatchToProps(dispatch: Dispatch): DispatchProps { + return { + setFocus: windowId => dispatch(Actions.setFocusedWindow(windowId)), + }; +} + +export default connect( + mapStateToProps, + mapDispatchToProps +)(FocusTarget); diff --git a/js/components/GenWindow/index.tsx b/js/components/GenWindow/index.tsx index e21b96d9..956182d7 100644 --- a/js/components/GenWindow/index.tsx +++ b/js/components/GenWindow/index.tsx @@ -3,11 +3,11 @@ import { connect } from "react-redux"; import classnames from "classnames"; import "../../../css/gen-window.css"; -import { SET_FOCUSED_WINDOW } from "../../actionTypes"; import { setWindowSize, closeWindow } from "../../actionCreators"; import { getWindowPixelSize } from "../../selectors"; import ResizeTarget from "../ResizeTarget"; import { AppState, WindowId, Dispatch } from "../../types"; +import FocusTarget from "../FocusTarget"; interface TextProps { children: string; @@ -41,10 +41,10 @@ interface OwnProps { windowId: WindowId; children: (windowSize: WindowSize) => React.ReactNode; title: string; + onKeyDown?(e: React.KeyboardEvent): void; } interface DispatchProps { - setFocus: (windowId: WindowId) => void; close: (windowId: WindowId) => void; setGenWindowSize: (windowId: WindowId, size: [number, number]) => void; } @@ -64,57 +64,62 @@ export const GenWindow = ({ children, close, title, - setFocus, windowId, windowSize, setGenWindowSize, width, height, + onKeyDown, }: Props) => { return ( -
setFocus(windowId)} - style={{ width, height }} - > -
-
-
-
-
- {title} + +
+
+
+
+
+
+ {title} +
+
+
+
+
close(windowId)} + /> +
-
-
-
-
close(windowId)} /> +
+
+
+
+
+ {children({ + width: width - CHROME_WIDTH, + height: height - CHROME_HEIGHT, + })} +
+
+
+
+
+
+
+
+ setGenWindowSize(windowId, size)} + id={"gen-resize-target"} + /> +
-
-
-
-
-
- {children({ - width: width - CHROME_WIDTH, - height: height - CHROME_HEIGHT, - })} -
-
-
-
-
-
-
-
- setGenWindowSize(windowId, size)} - id={"gen-resize-target"} - /> -
-
-
+ ); }; @@ -130,8 +135,6 @@ const mapStateToProps = (state: AppState, ownProps: OwnProps): StateProps => { const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => { return { - setFocus: (windowId: WindowId) => - dispatch({ type: SET_FOCUSED_WINDOW, window: windowId }), close: (windowId: WindowId) => dispatch(closeWindow(windowId)), setGenWindowSize: (windowId: WindowId, size: [number, number]) => dispatch(setWindowSize(windowId, size)), diff --git a/js/components/MainWindow/index.tsx b/js/components/MainWindow/index.tsx index 70795de8..4b8a0004 100644 --- a/js/components/MainWindow/index.tsx +++ b/js/components/MainWindow/index.tsx @@ -45,6 +45,7 @@ import { Dispatch, FilePicker, } from "../../types"; +import FocusTarget from "../FocusTarget"; interface StateProps { focused: WindowId; @@ -57,7 +58,6 @@ interface StateProps { } interface DispatchProps { - setFocus(): void; loadFilesFromReferences(files: FileList): void; scrollVolume(e: React.WheelEvent): void; toggleMainWindowShadeMode(): void; @@ -72,10 +72,6 @@ interface OwnProps { type Props = StateProps & DispatchProps & OwnProps; export class MainWindow extends React.Component { - _handleClick = () => { - this.props.setFocus(); - }; - render() { const { focused, @@ -105,65 +101,66 @@ export class MainWindow extends React.Component { -
- } - > - - - {mainShade && } - - - -
-
- - {!working &&
} +
+ } + > + + + {mainShade && } + + + +
+
+ + {!working &&
} +
+
+ -
- -
- - - - - - - -
- - -
- - - -
- - -
- +
+ + + + + + + +
+ + +
+ + + +
+ + +
+ + ); } @@ -188,8 +185,6 @@ const mapStateToProps = (state: AppState): StateProps => { const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => { return { - setFocus: () => - dispatch({ type: SET_FOCUSED_WINDOW, window: WINDOWS.MAIN }), loadFilesFromReferences: (files: FileList) => dispatch(loadFilesFromReferences(files)), toggleMainWindowShadeMode: () => dispatch(toggleMainWindowShadeMode()), @@ -200,5 +195,7 @@ const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => { }; export default connect( mapStateToProps, - mapDispatchToProps + mapDispatchToProps, + null, + { forwardRef: true } )(MainWindow); diff --git a/js/components/MilkdropWindow/PresetOverlay.tsx b/js/components/MilkdropWindow/PresetOverlay.tsx index 7051b138..69df81e2 100644 --- a/js/components/MilkdropWindow/PresetOverlay.tsx +++ b/js/components/MilkdropWindow/PresetOverlay.tsx @@ -52,9 +52,6 @@ interface DispatchProps { interface OwnProps { height: number; width: number; - onFocusedKeyDown( - cb: (e: React.KeyboardEvent) => void - ): () => void; } type Props = StateProps & DispatchProps & OwnProps; @@ -69,12 +66,6 @@ class PresetOverlay extends React.Component { } componentDidMount() { - this._disposable.add( - // Technically we should handle the case where this prop changes, but we don't. - // If we convert to hooks for this component, this would get much easier. - // _Also_ ideally we could avoid this prop all together. - this.props.onFocusedKeyDown(this._handleFocusedKeyboardInput) - ); const { currentPresetIndex } = this.props; if (currentPresetIndex != null) { this.setState({ @@ -188,7 +179,10 @@ class PresetOverlay extends React.Component { ); } return ( -
+
) => void - ): () => void; } type Props = StateProps & DispatchProps & OwnProps; -function useMilkdropKeyBindings(props: Props) { +function useKeyHandler(props: Props) { const { - onFocusedKeyDown, selectNextPreset, selectPreviousPreset, toggleRandomize, @@ -63,8 +59,8 @@ function useMilkdropKeyBindings(props: Props) { toggleCycling, } = props; // Handle keyboard events - useEffect(() => { - return onFocusedKeyDown(e => { + return useCallback( + (e: React.KeyboardEvent) => { switch (e.keyCode) { case 32: // spacebar selectNextPreset(); @@ -93,21 +89,21 @@ function useMilkdropKeyBindings(props: Props) { toggleCycling(); break; } - }); - }, [ - onFocusedKeyDown, - selectNextPreset, - selectPreviousPreset, - toggleRandomize, - togglePresetOverlay, - scheduleMilkdropMessage, - trackTitle, - toggleCycling, - ]); + }, + [ + selectNextPreset, + selectPreviousPreset, + toggleRandomize, + togglePresetOverlay, + scheduleMilkdropMessage, + trackTitle, + toggleCycling, + ] + ); } function Milkdrop(props: Props) { - useMilkdropKeyBindings(props); + const handleKeyDown = useKeyHandler(props); // Cycle presets useEffect(() => { @@ -139,19 +135,18 @@ function Milkdrop(props: Props) { } return ( - + {(windowSize: { width: number; height: number }) => { const size = props.fullscreen ? screenSize : windowSize; return ( - {props.overlay && ( - - )} + {props.overlay && } { render() { const { skinPlaylistStyle, - focusPlaylist, selected, playlistSize, playlistWindowPixelSize, @@ -109,69 +107,70 @@ class PlaylistWindow extends React.Component { id="playlist-window" className={classes} style={style} - onMouseDown={focusPlaylist} handleDrop={this._handleDrop} onWheel={this.props.scrollVolume} > -
-
- {showSpacers && ( -
- )} -
-
- {showSpacers && ( -
- )} -
-
-
-
-
-
-
-
-
- -
-
- -
-
-
-
- - - - -
-
-
- {showVisualizer && ( -
- {activateVisualizer && ( -
- -
- )} -
+ +
+
+ {showSpacers && ( +
)} - - -
-
- +
+
+ {showSpacers && ( +
+ )} +
+
+
+
+
-
+
+
+
+ +
+
+ +
+
+
+
+ + + + +
+
+
+ {showVisualizer && ( +
+ {activateVisualizer && ( +
+ +
+ )} +
+ )} + + +
+
+ +
+
+ ); } @@ -179,8 +178,6 @@ class PlaylistWindow extends React.Component { const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => { return { - focusPlaylist: () => - dispatch({ type: SET_FOCUSED_WINDOW, window: WINDOWS.PLAYLIST }), close: () => dispatch(closeWindow(WINDOWS.PLAYLIST)), toggleShade: () => dispatch(togglePlaylistShadeMode()), scrollUpFourTracks: () => dispatch(scrollUpFourTracks()),