From d6a1baece3abb0a0939867534ca00ecd659bb251 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sat, 29 Sep 2018 17:34:06 -0700 Subject: [PATCH] Refactor how we manage window positions --- js/actionCreators/index.ts | 9 ++- js/actionCreators/windows.ts | 131 +++++++++++++++----------------- js/actionTypes.ts | 3 +- js/components/WindowManager.tsx | 22 +----- js/reducers/windows.ts | 96 +++++++++++++---------- js/selectors.ts | 34 ++++++--- js/types.ts | 45 ++++------- js/utils.ts | 2 +- js/webampLazy.js | 57 +++++++------- 9 files changed, 189 insertions(+), 210 deletions(-) diff --git a/js/actionCreators/index.ts b/js/actionCreators/index.ts index ba35fce1..b1fab2a7 100644 --- a/js/actionCreators/index.ts +++ b/js/actionCreators/index.ts @@ -22,11 +22,12 @@ export { toggleWindow, updateWindowPositions, toggleMainWindowShadeMode, - windowsHaveBeenCentered, - centerWindowsIfNeeded, - resetWindowLayout, + centerWindowsInContainer, + centerWindowsInView, + resetWindowSizes, browserWindowSizeChanged, - ensureWindowsAreOnScreen + ensureWindowsAreOnScreen, + stackWindows } from "./windows"; export { play, diff --git a/js/actionCreators/windows.ts b/js/actionCreators/windows.ts index 829335f8..b8e27bbf 100644 --- a/js/actionCreators/windows.ts +++ b/js/actionCreators/windows.ts @@ -9,18 +9,14 @@ import { CLOSE_WINDOW, TOGGLE_WINDOW_SHADE_MODE, SET_WINDOW_VISIBILITY, - WINDOWS_HAVE_BEEN_CENTERED, BROWSER_WINDOW_SIZE_CHANGED, - RESET_WINDOW_LAYOUT + RESET_WINDOW_SIZES } from "../actionTypes"; import { getPositionDiff, SizeDiff } from "../resizeUtils"; import { applyDiff } from "../snapUtils"; import { Action, Dispatchable, WindowId, WindowPositions } from "../types"; -import { WINDOW_HEIGHT, WINDOW_WIDTH } from "../constants"; -import { calculateBoundingBox } from "../utils"; - // Dispatch an action and, if needed rearrange the windows to preserve // the existing edge relationship. // @@ -53,7 +49,7 @@ function withWindowGraphIntegrity(action: Action): Dispatchable { applyDiff(position, positionDiff[key]) ); - dispatch(updateWindowPositions(newPositions, false)); + dispatch(updateWindowPositions(newPositions)); }; } @@ -106,81 +102,64 @@ export function toggleWindow(windowId: WindowId): Dispatchable { } export function updateWindowPositions( - positions: WindowPositions, - center: boolean + positions: WindowPositions ): Dispatchable { - return { type: UPDATE_WINDOW_POSITIONS, positions, center }; + return { type: UPDATE_WINDOW_POSITIONS, positions }; } -export function windowsHaveBeenCentered(): Dispatchable { - return { type: WINDOWS_HAVE_BEEN_CENTERED }; +export function centerWindowsInContainer(container: HTMLElement): Dispatchable { + const { left, top } = container.getBoundingClientRect(); + const { scrollWidth: width, scrollHeight: height } = container; + return centerWindows({ left, top, width, height }); } -export function centerWindowsIfNeeded(container: HTMLElement): Dispatchable { +export function centerWindowsInView(): Dispatchable { + return centerWindows({ + left: window.scrollX, + top: window.scrollY, + width: window.innerWidth, + height: window.innerHeight + }); +} + +export function centerWindows(box: { + left: number; + top: number; + width: number; + height: number; +}): Dispatchable { return (dispatch, getState) => { const state = getState(); - const { centerRequested } = state.windows; - if (!centerRequested) { - return; - } - const genWindows = Selectors.getGenWindows(state); const windowsInfo = Selectors.getWindowsInfo(state); const getOpen = Selectors.getWindowOpen(state); - const getPixelSize = Selectors.getWindowPixelSize(state); - const rect = container.getBoundingClientRect(); + const { top, left, width, height } = box; - const offsetLeft = rect.left + window.scrollX; - const offsetTop = rect.top + window.scrollY; - const width = container.scrollWidth; - const height = container.scrollHeight; + const offsetLeft = left + window.scrollX; + const offsetTop = top + window.scrollY; - if (windowsInfo.some(w => w.x == null || w.y == null)) { - // Some windows do not have an initial position, so we'll come up - // with your own layout. - const windowPositions: WindowPositions = {}; - const keys: string[] = Object.keys(genWindows).filter( - windowId => genWindows[windowId].open - ); - const totalHeight = keys.reduce((height, key) => { - return height + getPixelSize(key).height; - }, 0); - const globalOffsetLeft = Math.max(0, width / 2 - WINDOW_WIDTH / 2); - const globalOffsetTop = Math.max(0, height / 2 - totalHeight / 2); - let offset = 0; - keys.forEach(key => { - windowPositions[key] = { - x: Math.ceil(offsetLeft + globalOffsetLeft), - y: Math.ceil(offsetTop + (globalOffsetTop + offset)) - }; - offset += getPixelSize(key).height; - }); - dispatch(updateWindowPositions(windowPositions, false)); - } else { - // A layout has been suplied. We will compute the bounding box and - // center the given layout. - const bounding = calculateBoundingBox( - windowsInfo.filter(w => getOpen(w.key)) - ); + // A layout has been suplied. We will compute the bounding box and + // center the given layout. + const bounding = Utils.calculateBoundingBox( + windowsInfo.filter(w => getOpen(w.key)) + ); - const boxHeight = bounding.bottom - bounding.top; - const boxWidth = bounding.right - bounding.left; + const boxHeight = bounding.bottom - bounding.top; + const boxWidth = bounding.right - bounding.left; - const move = { - x: Math.ceil(offsetLeft - bounding.left + (width - boxWidth) / 2), - y: Math.ceil(offsetTop - bounding.top + (height - boxHeight) / 2) - }; + const move = { + x: Math.ceil(offsetLeft - bounding.left + (width - boxWidth) / 2), + y: Math.ceil(offsetTop - bounding.top + (height - boxHeight) / 2) + }; - const newPositions = windowsInfo.reduce( - (pos, w) => ({ - ...pos, - [w.key]: { x: move.x + w.x, y: move.y + w.y } - }), - {} - ); + const newPositions = windowsInfo.reduce( + (pos, w) => ({ + ...pos, + [w.key]: { x: move.x + w.x, y: move.y + w.y } + }), + {} + ); - dispatch(updateWindowPositions(newPositions, false)); - } - dispatch(windowsHaveBeenCentered()); + dispatch(updateWindowPositions(newPositions)); }; } @@ -189,8 +168,16 @@ export function browserWindowSizeChanged() { return { type: BROWSER_WINDOW_SIZE_CHANGED, height, width }; } -export function resetWindowLayout(): Dispatchable { - return { type: RESET_WINDOW_LAYOUT }; +export function resetWindowSizes(): Dispatchable { + return { type: RESET_WINDOW_SIZES }; +} + +export function stackWindows(): Dispatchable { + return (dispatch, getState) => { + dispatch( + updateWindowPositions(Selectors.getStackedLayoutPositions(getState())) + ); + }; } export function ensureWindowsAreOnScreen(): Dispatchable { @@ -200,7 +187,7 @@ export function ensureWindowsAreOnScreen(): Dispatchable { const windowsInfo = Selectors.getWindowsInfo(state); const getOpen = Selectors.getWindowOpen(state); const { height, width } = Utils.getWindowSize(); - const bounding = calculateBoundingBox( + const bounding = Utils.calculateBoundingBox( windowsInfo.filter(w => getOpen(w.key)) ); const positions = Selectors.getWindowPositions(state); @@ -240,13 +227,15 @@ export function ensureWindowsAreOnScreen(): Dispatchable { y: position.y - moveY })); - dispatch(updateWindowPositions(newPositions, false)); + dispatch(updateWindowPositions(newPositions)); return; } // TODO: Try moving the individual groups to try to fit them in // I give up. Just reset everything. - dispatch(resetWindowLayout()); + dispatch(resetWindowSizes()); + dispatch(stackWindows()); + dispatch(centerWindowsInView()); }; } diff --git a/js/actionTypes.ts b/js/actionTypes.ts index 0714e56d..2267ba52 100644 --- a/js/actionTypes.ts +++ b/js/actionTypes.ts @@ -72,6 +72,5 @@ export const SET_WINDOW_VISIBILITY = "SET_WINDOW_VISIBILITY"; export const LOADING = "LOADING"; export const CLOSE_REQUESTED = "CLOSE_REQUESTED"; export const LOAD_SERIALIZED_STATE = "LOAD_SERIALIZED_STATE"; -export const WINDOWS_HAVE_BEEN_CENTERED = "WINDOWS_HAVE_BEEN_CENTERED"; -export const RESET_WINDOW_LAYOUT = "RESET_WINDOW_LAYOUT"; +export const RESET_WINDOW_SIZES = "RESET_WINDOW_SIZES"; export const BROWSER_WINDOW_SIZE_CHANGED = "BROWSER_WINDOW_SIZE_CHANGED"; diff --git a/js/components/WindowManager.tsx b/js/components/WindowManager.tsx index 5c6a8951..c6a95a32 100644 --- a/js/components/WindowManager.tsx +++ b/js/components/WindowManager.tsx @@ -4,11 +4,7 @@ import { connect } from "react-redux"; import { Box, Diff } from "../snapUtils"; import * as SnapUtils from "../snapUtils"; import * as Selectors from "../selectors"; -import { - updateWindowPositions, - windowsHaveBeenCentered, - centerWindowsIfNeeded -} from "../actionCreators"; +import { updateWindowPositions } from "../actionCreators"; import { WindowInfo, Dispatch, @@ -24,7 +20,6 @@ const abuts = (a: Box, b: Box) => { }; interface Props { - centerWindowsIfNeeded(container: HTMLElement): void; container: HTMLElement; windowsInfo: WindowInfo[]; browserWindowSize: { height: number; width: number }; @@ -34,14 +29,6 @@ interface Props { } class WindowManager extends React.Component { - componentDidMount() { - this.props.centerWindowsIfNeeded(this.props.container); - } - - componentDidUpdate() { - this.props.centerWindowsIfNeeded(this.props.container); - } - movingAndStationaryNodes(key: WindowId): [WindowInfo[], WindowInfo[]] { const windows = this.props.windowsInfo.filter( w => @@ -163,11 +150,8 @@ const mapStateToProps = (state: AppState) => ({ const mapDispatchToProps = (dispatch: Dispatch) => { return { - updateWindowPositions: (positions: WindowPositions, centered: boolean) => - dispatch(updateWindowPositions(positions, centered)), - windowsHaveBeenCentered: () => dispatch(windowsHaveBeenCentered()), - centerWindowsIfNeeded: (container: HTMLElement) => - dispatch(centerWindowsIfNeeded(container)) + updateWindowPositions: (positions: WindowPositions) => + dispatch(updateWindowPositions(positions)) }; }; diff --git a/js/reducers/windows.ts b/js/reducers/windows.ts index ef1b002b..51f7bab8 100644 --- a/js/reducers/windows.ts +++ b/js/reducers/windows.ts @@ -1,4 +1,4 @@ -import { WebampWindow, WindowPositions, Action, WindowId } from "../types"; +import { Action, WindowId } from "../types"; import { WINDOWS } from "../constants"; import { SET_FOCUSED_WINDOW, @@ -10,16 +10,44 @@ import { WINDOW_SIZE_CHANGED, TOGGLE_WINDOW_SHADE_MODE, LOAD_SERIALIZED_STATE, - RESET_WINDOW_LAYOUT, - BROWSER_WINDOW_SIZE_CHANGED + BROWSER_WINDOW_SIZE_CHANGED, + RESET_WINDOW_SIZES } from "../actionTypes"; import * as Utils from "../utils"; +export interface WindowPosition { + x: number; + y: number; +} + +export type WindowPositions = { + [windowId: string]: WindowPosition; +}; + +export interface WebampWindow { + title: string; + size: [number, number]; + open: boolean; + hidden: boolean; + shade?: boolean; + canResize: boolean; + canShade: boolean; + canDouble: boolean; + generic: boolean; + hotkey?: string; + position: WindowPosition; +} + +export interface WindowInfo { + key: WindowId; + height: number; + width: number; + x: number; + y: number; +} export interface WindowsState { focused: string; - centerRequested: boolean; genWindows: { [name: string]: WebampWindow }; - positions: WindowPositions; browserWindowSize: { height: number; width: number }; } @@ -28,18 +56,16 @@ interface SerializedWindow { open: boolean; hidden: boolean; shade: boolean; + position: WindowPosition; } export interface WindowsSerializedStateV1 { genWindows: { [windowId: string]: SerializedWindow }; focused: string; - centerRequested: boolean; - positions: WindowPositions; } const defaultWindowsState: WindowsState = { focused: WINDOWS.MAIN, - centerRequested: false, genWindows: { // TODO: Remove static capabilites and derive them from ids/generic main: { @@ -52,7 +78,8 @@ const defaultWindowsState: WindowsState = { canShade: true, canDouble: true, generic: false, - hotkey: "Alt+W" + hotkey: "Alt+W", + position: { x: 0, y: 0 } }, equalizer: { title: "Equalizer", @@ -64,7 +91,8 @@ const defaultWindowsState: WindowsState = { canShade: true, canDouble: true, generic: false, - hotkey: "Alt+G" + hotkey: "Alt+G", + position: { x: 0, y: 0 } }, playlist: { title: "Playlist Editor", @@ -76,10 +104,10 @@ const defaultWindowsState: WindowsState = { canShade: true, canDouble: false, generic: false, - hotkey: "Alt+E" + hotkey: "Alt+E", + position: { x: 0, y: 0 } } }, - positions: {}, browserWindowSize: { width: 0, height: 0 } }; @@ -158,7 +186,8 @@ const windows = ( canShade: false, canResize: true, canDouble: false, - generic: true + generic: true, + position: { x: 0, y: 0 } } } }; @@ -182,30 +211,25 @@ const windows = ( case UPDATE_WINDOW_POSITIONS: return { ...state, - positions: { - ...state.positions, - ...action.positions - }, - centerRequested: action.center + genWindows: Utils.objectMap(state.genWindows, (w, windowId) => { + const newPosition = action.positions[windowId]; + if (newPosition == null) { + return w; + } + return { ...w, position: newPosition }; + }) }; - case RESET_WINDOW_LAYOUT: + case RESET_WINDOW_SIZES: return { ...state, genWindows: Utils.objectMap(state.genWindows, w => ({ ...w, // Not sure why TypeScript can't figure this out for itself. size: [0, 0] as [number, number] - })), - positions: {}, - centerRequested: true + })) }; case LOAD_SERIALIZED_STATE: { - const { - genWindows, - positions, - centerRequested, - focused - } = action.serializedState.windows; + const { genWindows, focused } = action.serializedState.windows; return { ...state, genWindows: Utils.objectMap(state.genWindows, (w, windowId) => { @@ -215,15 +239,6 @@ const windows = ( } return { ...w, ...serializedW }; }), - // Note: We iterate genWindows here, since the positions object may be empty - positions: Utils.objectMap(state.genWindows, (position, windowId) => { - const serializedPosition = positions[windowId]; - if (serializedPosition == null) { - return state.positions[windowId]; - } - return serializedPosition; - }), - centerRequested, focused }; } @@ -247,12 +262,11 @@ export function getSerializedState( size: w.size, open: w.open, hidden: w.hidden, - shade: w.shade || false + shade: w.shade || false, + position: w.position }; }), - focused: state.focused, - positions: state.positions, - centerRequested: state.centerRequested + focused: state.focused }; } diff --git a/js/selectors.ts b/js/selectors.ts index 27122796..d6c6b48d 100644 --- a/js/selectors.ts +++ b/js/selectors.ts @@ -5,7 +5,8 @@ import { WindowId, WindowInfo, LoadedURLTrack, - SerializedStateV1 + SerializedStateV1, + WindowPositions } from "./types"; import { createSelector } from "reselect"; import * as Utils from "./utils"; @@ -321,10 +322,6 @@ export const getPlaylistURL = createSelector( }) ); -export function getWindowPositions(state: AppState) { - return state.windows.positions; -} - const WINDOW_HEIGHT = 116; const SHADE_WINDOW_HEIGHT = 14; @@ -361,6 +358,11 @@ export const getGenWindows = (state: AppState) => { return state.windows.genWindows; }; +export const getWindowPositions = createSelector( + getGenWindows, + (windows): WindowPositions => Utils.objectMap(windows, w => w.position) +); + export function getDoubled(state: AppState) { return state.display.doubled; } @@ -377,6 +379,7 @@ export const getWindowPixelSize = createSelector(getWindowSizes, sizes => { return (windowId: WindowId) => sizes[windowId]; }); +// TODO: Now that both size and position are stored on genWindows this seems a bit silly. export const getWindowsInfo = createSelector( getWindowSizes, getWindowPositions, @@ -419,12 +422,25 @@ export function getEqualizerEnabled(state: AppState): boolean { return state.equalizer.on; } -export function getCenterRequested(state: AppState): boolean { - return state.windows.centerRequested; -} - export function getBrowserWindowSize( state: AppState ): { height: number; width: number } { return state.windows.browserWindowSize; } + +export const getOpenWindows = createSelector(getGenWindows, genWindows => + Utils.objectFilter(genWindows, w => w.open) +); + +export const getStackedLayoutPositions = createSelector( + getOpenWindows, + getDoubled, + (openWindows, doubled): WindowPositions => { + let offset = 0; + return Utils.objectMap(openWindows, w => { + const position = { x: 0, y: offset }; + offset += getWPixelSize(w, doubled).height; + return position; + }); + } +); diff --git a/js/types.ts b/js/types.ts index d1ac6d8f..c20aa1b0 100644 --- a/js/types.ts +++ b/js/types.ts @@ -3,13 +3,24 @@ import { SettingsState } from "./reducers/settings"; import { UserInputState } from "./reducers/userInput"; import { MediaState, MediaSerializedStateV1 } from "./reducers/media"; import { DisplayState, DisplaySerializedStateV1 } from "./reducers/display"; -import { WindowsState, WindowsSerializedStateV1 } from "./reducers/windows"; +import { + WindowsState, + WindowsSerializedStateV1, + WindowPositions +} from "./reducers/windows"; import { EqualizerState, EqualizerSerializedStateV1 } from "./reducers/equalizer"; import { NetworkState } from "./reducers/network"; +export { + WebampWindow, + WindowInfo, + WindowPosition, + WindowPositions +} from "./reducers/windows"; + export type Skin = { url: string; name: string; @@ -56,13 +67,6 @@ type SkinData = { skinGenLetterWidths: GenLetterWidths; }; -interface WindowPosition { - x: number; - y: number; -} -export type WindowPositions = { - [windowId: string]: WindowPosition; -}; export type Action = | { type: "@@init"; @@ -256,7 +260,6 @@ export type Action = | { type: "UPDATE_WINDOW_POSITIONS"; positions: WindowPositions; - center: boolean; } | { type: "CLICKED_TRACK"; @@ -342,7 +345,6 @@ export type Action = | { type: "MINIMIZE_WINAMP"; } - | { type: "WINDOWS_HAVE_BEEN_CENTERED" } | { type: "CLOSE_REQUESTED"; cancel: () => void; @@ -351,30 +353,9 @@ export type Action = type: "LOAD_SERIALIZED_STATE"; serializedState: SerializedStateV1; } - | { type: "RESET_WINDOW_LAYOUT" } + | { type: "RESET_WINDOW_SIZES" } | { type: "BROWSER_WINDOW_SIZE_CHANGED"; height: number; width: number }; -export interface WebampWindow { - title: string; - size: [number, number]; - open: boolean; - hidden: boolean; - shade?: boolean; - canResize: boolean; - canShade: boolean; - canDouble: boolean; - generic: boolean; - hotkey?: string; -} - -export interface WindowInfo { - key: WindowId; - height: number; - width: number; - x: number; - y: number; -} - export type MediaTagRequestStatus = | "INITIALIZED" | "FAILED" diff --git a/js/utils.ts b/js/utils.ts index 33d8002c..daf3eaf3 100644 --- a/js/utils.ts +++ b/js/utils.ts @@ -309,7 +309,7 @@ export function objectMap( export function objectFilter( obj: { [key: string]: V }, predicate: (value: V, key: string) => boolean -) { +): { [key: string]: V } { // TODO: Could return the original reference if no values change return Object.keys(obj).reduce((newObj: { [key: string]: V }, key) => { if (predicate(obj[key], key)) { diff --git a/js/webampLazy.js b/js/webampLazy.js index 82806c40..26376679 100644 --- a/js/webampLazy.js +++ b/js/webampLazy.js @@ -7,21 +7,8 @@ import App from "./components/App"; import { bindHotkeys } from "./hotkeys"; import Media from "./media"; import * as Selectors from "./selectors"; -import { - setSkinFromUrl, - loadMediaFiles, - setWindowSize, - play, - pause, - seekBackward, - seekForward, - next, - previous, - updateWindowPositions, - loadSerializedState, - ensureWindowsAreOnScreen, - browserWindowSizeChanged -} from "./actionCreators"; +import * as Actions from "./actionCreators"; + import { LOAD_STYLE } from "./constants"; import * as Utils from "./utils"; @@ -130,14 +117,14 @@ class Winamp { this.store.dispatch({ type: NETWORK_DISCONNECTED }) ); - this.store.dispatch(browserWindowSizeChanged()); + this.store.dispatch(Actions.browserWindowSizeChanged()); window.addEventListener("resize", () => { - this.store.dispatch(ensureWindowsAreOnScreen()); - this.store.dispatch(browserWindowSizeChanged()); + this.store.dispatch(Actions.browserWindowSizeChanged()); + this.store.dispatch(Actions.ensureWindowsAreOnScreen()); }); if (initialSkin) { - this.store.dispatch(setSkinFromUrl(initialSkin.url)); + this.store.dispatch(Actions.setSkinFromUrl(initialSkin.url)); } else { // We are using the default skin. this.store.dispatch({ type: LOADED }); @@ -157,14 +144,19 @@ class Winamp { } const layout = options.__initialWindowLayout; - if (layout != null) { + if (layout == null) { + this.store.dispatch(Actions.stackWindows()); + } else { Utils.objectForEach(layout, (w, windowId) => { if (w.size != null) { - this.store.dispatch(setWindowSize(windowId, w.size)); + this.store.dispatch(Actions.setWindowSize(windowId, w.size)); } }); this.store.dispatch( - updateWindowPositions(Utils.objectMap(layout, w => w.position), true) + Actions.updateWindowPositions( + Utils.objectMap(layout, w => w.position), + true + ) ); } @@ -174,38 +166,40 @@ class Winamp { } play() { - this.store.dispatch(play()); + this.store.dispatch(Actions.play()); } pause() { - this.store.dispatch(pause()); + this.store.dispatch(Actions.pause()); } seekBackward(seconds) { - this.store.dispatch(seekBackward(seconds)); + this.store.dispatch(Actions.seekBackward(seconds)); } seekForward(seconds) { - this.store.dispatch(seekForward(seconds)); + this.store.dispatch(Actions.seekForward(seconds)); } nextTrack() { - this.store.dispatch(next()); + this.store.dispatch(Actions.next()); } previousTrack() { - this.store.dispatch(previous()); + this.store.dispatch(Actions.previous()); } // Append this array of tracks to the end of the current playlist. appendTracks(tracks) { const nextIndex = Selectors.getTrackCount(this.store.getState()); - this.store.dispatch(loadMediaFiles(tracks, LOAD_STYLE.BUFFER, nextIndex)); + this.store.dispatch( + Actions.loadMediaFiles(tracks, LOAD_STYLE.BUFFER, nextIndex) + ); } // Replace any existing tracks with this array of tracks, and begin playing. setTracksToPlay(tracks) { - this.store.dispatch(loadMediaFiles(tracks, LOAD_STYLE.PLAY)); + this.store.dispatch(Actions.loadMediaFiles(tracks, LOAD_STYLE.PLAY)); } onWillClose(cb) { @@ -241,7 +235,7 @@ class Winamp { } __loadSerializedState(serializedState) { - this.store.dispatch(loadSerializedState(serializedState)); + this.store.dispatch(Actions.loadSerializedState(serializedState)); } __getSerializedState() { @@ -253,6 +247,7 @@ class Winamp { } async renderWhenReady(node) { + this.store.dispatch(Actions.centerWindowsInContainer(node)); await this.skinIsLoaded(); const genWindowComponents = {}; this.genWindows.forEach(w => {