From 426e1a433bfb2d20193f39757d63ff0179b3d6fe Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Tue, 24 May 2022 16:37:24 -0400 Subject: [PATCH 1/3] Make drag-and-drop smoother --- package.json | 1 - src/room/InCallView.jsx | 24 +---- src/video-grid/VideoGrid.jsx | 189 ++++++++++++++++------------------- yarn.lock | 9 +- 4 files changed, 88 insertions(+), 135 deletions(-) diff --git a/package.json b/package.json index c39aa9f..fbfcfb2 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,6 @@ "classnames": "^2.3.1", "color-hash": "^2.0.1", "events": "^3.3.0", - "lodash-move": "^1.1.1", "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#aa0d3bd1f5a006d151f826e6b8c5f286abb6e960", "mermaid": "^8.13.8", "normalize.css": "^8.0.1", diff --git a/src/room/InCallView.jsx b/src/room/InCallView.jsx index 9e1fab0..a474963 100644 --- a/src/room/InCallView.jsx +++ b/src/room/InCallView.jsx @@ -104,23 +104,6 @@ export function InCallView({ return participants; }, [userMediaFeeds, activeSpeaker, screenshareFeeds, layout]); - const onFocusTile = useCallback( - (tiles, focusedTile) => { - if (layout === "freedom") { - return tiles.map((tile) => { - if (tile === focusedTile) { - return { ...tile, focused: !tile.focused }; - } - - return tile; - }); - } else { - return tiles; - } - }, - [layout, setLayout] - ); - const renderAvatar = useCallback( (roomMember, width, height) => { const avatarUrl = roomMember.user?.avatarUrl; @@ -160,12 +143,7 @@ export function InCallView({

Waiting for other participants...

) : ( - + {({ item, ...rest }) => ( { - if (is1on1Freedom && a.item.isLocal !== b.item.isLocal) { - return (b.item.isLocal ? 1 : 0) - (a.item.isLocal ? 1 : 0); - } else if (a.focused !== b.focused) { - return (b.focused ? 1 : 0) - (a.focused ? 1 : 0); - } else if (a.presenter !== b.presenter) { - return (b.presenter ? 1 : 0) - (a.presenter ? 1 : 0); - } + const orderedTiles = new Array(tiles.length); + tiles.forEach((tile) => (orderedTiles[tile.order] = tile)); - return 0; - }); + orderedTiles.forEach((tile) => + (tile.focused + ? focusedTiles + : tile.presenter + ? presenterTiles + : otherTiles + ).push(tile) + ); + + [...focusedTiles, ...presenterTiles, ...otherTiles].forEach( + (tile, i) => (tile.order = i) + ); } -export function VideoGrid({ - items, - layout, - onFocusTile, - disableAnimations, - children, -}) { +export function VideoGrid({ items, layout, disableAnimations, children }) { // Place the PiP in the bottom right corner by default const [pipXRatio, setPipXRatio] = useState(1); const [pipYRatio, setPipYRatio] = useState(1); - const [{ tiles, tilePositions, scrollPosition }, setTileState] = useState({ + const [{ tiles, tilePositions }, setTileState] = useState({ tiles: [], tilePositions: [], - scrollPosition: 0, }); + const [scrollPosition, setScrollPosition] = useState(0); const draggingTileRef = useRef(null); const lastTappedRef = useRef({}); const lastLayoutRef = useRef(layout); @@ -646,7 +645,7 @@ export function VideoGrid({ useEffect(() => { setTileState(({ tiles, ...rest }) => { const newTiles = []; - const removedTileKeys = []; + const removedTileKeys = new Set(); for (const tile of tiles) { let item = items.find((item) => item.id === tile.key); @@ -656,7 +655,7 @@ export function VideoGrid({ if (!item) { remove = true; item = tile.item; - removedTileKeys.push(tile.key); + removedTileKeys.add(tile.key); } let focused; @@ -671,6 +670,7 @@ export function VideoGrid({ newTiles.push({ key: item.id, + order: tile.order, item, remove, focused, @@ -691,6 +691,7 @@ export function VideoGrid({ const newTile = { key: item.id, + order: existingTile?.order ?? newTiles.length, item, remove: false, focused: layout === "spotlight" && item.focused, @@ -706,22 +707,19 @@ export function VideoGrid({ } } - sortTiles(layout, newTiles); + reorderTiles(newTiles); - if (removedTileKeys.length > 0) { + if (removedTileKeys.size > 0) { setTimeout(() => { if (!isMounted.current) { return; } setTileState(({ tiles, ...rest }) => { - const newTiles = tiles.filter( - (tile) => !removedTileKeys.includes(tile.key) - ); - - // TODO: When we remove tiles, we reuse the order of the tiles vs calling sort on the - // items array. This can cause the local feed to display large in the room. - // To fix this we need to move to using a reducer and sorting the input items + const newTiles = tiles + .filter((tile) => !removedTileKeys.has(tile.key)) + .map((tile) => ({ ...tile })); // clone before reordering + reorderTiles(newTiles); const presenterTileCount = newTiles.reduce( (count, tile) => count + (tile.focused ? 1 : 0), @@ -771,7 +769,7 @@ export function VideoGrid({ const animate = useCallback( (tiles) => (tileIndex) => { const tile = tiles[tileIndex]; - const tilePosition = tilePositions[tileIndex]; + const tilePosition = tilePositions[tile.order]; const draggingTile = draggingTileRef.current; const dragging = draggingTile && tile.key === draggingTile.key; const remove = tile.remove; @@ -830,6 +828,9 @@ export function VideoGrid({ reset: false, immediate: (key) => disableAnimations || key === "zIndex" || key === "shadow", + // If we just stopped dragging a tile, give it time for its animation + // to settle before pushing its z-index back down + delay: (key) => (key === "zIndex" ? 500 : 0), }; } }, @@ -854,43 +855,25 @@ export function VideoGrid({ lastTappedRef.current[tileKey] = 0; const tile = tiles.find((tile) => tile.key === tileKey); - - if (!tile) { - return; - } - + if (!tile || layout !== "freedom") return; const item = tile.item; - setTileState((state) => { + setTileState(({ tiles, ...state }) => { let presenterTileCount = 0; + const newTiles = tiles.map((tile) => { + let newTile = { ...tile }; // clone before reordering - let newTiles; - - if (onFocusTile) { - newTiles = onFocusTile(state.tiles, tile); - - for (const tile of newTiles) { - if (tile.focused) { - presenterTileCount++; - } + if (tile.item === item) { + newTile.focused = !tile.focused; + } + if (newTile.focused) { + presenterTileCount++; } - } else { - newTiles = state.tiles.map((tile) => { - let newTile = tile; - if (tile.item === item) { - newTile = { ...tile, focused: !tile.focused }; - } + return newTile; + }); - if (newTile.focused) { - presenterTileCount++; - } - - return newTile; - }); - } - - sortTiles(layout, newTiles); + reorderTiles(newTiles); return { ...state, @@ -907,7 +890,7 @@ export function VideoGrid({ }; }); }, - [tiles, gridBounds, onFocusTile, layout] + [tiles, gridBounds, layout] ); const bindTile = useDrag( @@ -923,17 +906,18 @@ export function VideoGrid({ const dragTileIndex = tiles.findIndex((tile) => tile.key === key); const dragTile = tiles[dragTileIndex]; - const dragTilePosition = tilePositions[dragTileIndex]; - - let newTiles = tiles; + const dragTilePosition = tilePositions[dragTile.order]; const cursorPosition = [xy[0] - gridBounds.left, xy[1] - gridBounds.top]; - if (layout === "freedom" && tiles.length === 2) { - // We're in 1:1 mode, so only the local tile should be draggable - if (dragTileIndex !== 0) return; + let newTiles = tiles; - // Only update the position on the last event + if (tiles.length === 2) { + // We're in 1:1 mode, so only the local tile should be draggable + if (!dragTile.item.isLocal) return; + + // Position should only update on the very last event, to avoid + // compounding the offset on every drag event if (last) { const remotePosition = tilePositions[1]; @@ -959,37 +943,40 @@ export function VideoGrid({ setPipXRatio(Math.max(0, Math.min(1, newPipXRatio))); setPipYRatio(Math.max(0, Math.min(1, newPipYRatio))); } - } - - for ( - let hoverTileIndex = 0; - hoverTileIndex < tiles.length; - hoverTileIndex++ - ) { - const hoverTile = tiles[hoverTileIndex]; - const hoverTilePosition = tilePositions[hoverTileIndex]; - - if (hoverTile.key === key) { - continue; - } - - if (isInside(cursorPosition, hoverTilePosition)) { - newTiles = moveArrItem(tiles, dragTileIndex, hoverTileIndex); + } else { + const hoverTile = tiles.find( + (tile) => + tile.key !== key && + isInside(cursorPosition, tilePositions[tile.order]) + ); + if (hoverTile) { + // Shift the tiles into their new order newTiles = newTiles.map((tile) => { - if (tile === hoverTile) { - return { ...tile, focused: dragTile.focused }; - } else if (tile === dragTile) { - return { ...tile, focused: hoverTile.focused }; + let order = tile.order; + if (order < dragTile.order) { + if (order >= hoverTile.order) order++; + } else if (order > dragTile.order) { + if (order <= hoverTile.order) order--; } else { - return tile; + order = hoverTile.order; } + + let focused; + if (tile === hoverTile) { + focused = dragTile.focused; + } else if (tile === dragTile) { + focused = hoverTile.focused; + } else { + focused = tile.focused; + } + + return { ...tile, order, focused }; }); - sortTiles(layout, newTiles); + reorderTiles(newTiles); setTileState((state) => ({ ...state, tiles: newTiles })); - break; } } @@ -1037,13 +1024,9 @@ export function VideoGrid({ : gridBounds.height - lastTile.y - lastTile.height - GAP; } - setTileState((state) => ({ - ...state, - scrollPosition: Math.min( - Math.max(movement + state.scrollPosition, min), - 0 - ), - })); + setScrollPosition((scrollPosition) => + Math.min(Math.max(movement + scrollPosition, min), 0) + ); }, [layout, gridBounds, tilePositions] ); @@ -1060,7 +1043,7 @@ export function VideoGrid({
{springs.map(({ shadow, ...style }, i) => { const tile = tiles[i]; - const tilePosition = tilePositions[i]; + const tilePosition = tilePositions[tile.order]; return children({ ...bindTile(tile.key), diff --git a/yarn.lock b/yarn.lock index 44c9a4e..74d88f4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8461,13 +8461,6 @@ locate-path@^6.0.0: dependencies: p-locate "^5.0.0" -lodash-move@^1.1.1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/lodash-move/-/lodash-move-1.1.1.tgz#59f76e0f1ac57e6d8683f531bec07c5b6ea4e348" - integrity sha1-WfduDxrFfm2Gg/UxvsB8W26k40g= - dependencies: - lodash "^4.6.1" - lodash.curry@^4.0.1: version "4.1.1" resolved "https://registry.yarnpkg.com/lodash.curry/-/lodash.curry-4.1.1.tgz#248e36072ede906501d75966200a86dab8b23170" @@ -8493,7 +8486,7 @@ lodash.uniq@4.5.0: resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773" integrity sha1-0CJTc662Uq3BvILklFM5qEJ1R3M= -lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.6.1: +lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21: version "4.17.21" resolved "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz" integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== From 266861bdad45f869cb9b7ddbed4154921b0ffaf8 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Tue, 24 May 2022 16:54:33 -0400 Subject: [PATCH 2/3] Fix order of tiles in 1:1 layout --- src/video-grid/VideoGrid.jsx | 47 ++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/video-grid/VideoGrid.jsx b/src/video-grid/VideoGrid.jsx index ba08c74..a0db691 100644 --- a/src/video-grid/VideoGrid.jsx +++ b/src/video-grid/VideoGrid.jsx @@ -603,26 +603,31 @@ function getSubGridPositions( return newTilePositions; } -function reorderTiles(tiles) { - const focusedTiles = []; - const presenterTiles = []; - const otherTiles = []; +function reorderTiles(tiles, layout) { + if (layout === "freedom" && tiles.length === 2) { + // 1:1 layout + tiles.forEach((tile) => tile.order = tile.item.isLocal ? 0 : 1); + } else { + const focusedTiles = []; + const presenterTiles = []; + const otherTiles = []; - const orderedTiles = new Array(tiles.length); - tiles.forEach((tile) => (orderedTiles[tile.order] = tile)); + const orderedTiles = new Array(tiles.length); + tiles.forEach((tile) => (orderedTiles[tile.order] = tile)); - orderedTiles.forEach((tile) => - (tile.focused - ? focusedTiles - : tile.presenter - ? presenterTiles - : otherTiles - ).push(tile) - ); + orderedTiles.forEach((tile) => + (tile.focused + ? focusedTiles + : tile.presenter + ? presenterTiles + : otherTiles + ).push(tile) + ); - [...focusedTiles, ...presenterTiles, ...otherTiles].forEach( - (tile, i) => (tile.order = i) - ); + [...focusedTiles, ...presenterTiles, ...otherTiles].forEach( + (tile, i) => (tile.order = i) + ); + } } export function VideoGrid({ items, layout, disableAnimations, children }) { @@ -707,7 +712,7 @@ export function VideoGrid({ items, layout, disableAnimations, children }) { } } - reorderTiles(newTiles); + reorderTiles(newTiles, layout); if (removedTileKeys.size > 0) { setTimeout(() => { @@ -719,7 +724,7 @@ export function VideoGrid({ items, layout, disableAnimations, children }) { const newTiles = tiles .filter((tile) => !removedTileKeys.has(tile.key)) .map((tile) => ({ ...tile })); // clone before reordering - reorderTiles(newTiles); + reorderTiles(newTiles, layout); const presenterTileCount = newTiles.reduce( (count, tile) => count + (tile.focused ? 1 : 0), @@ -873,7 +878,7 @@ export function VideoGrid({ items, layout, disableAnimations, children }) { return newTile; }); - reorderTiles(newTiles); + reorderTiles(newTiles, layout); return { ...state, @@ -974,7 +979,7 @@ export function VideoGrid({ items, layout, disableAnimations, children }) { return { ...tile, order, focused }; }); - reorderTiles(newTiles); + reorderTiles(newTiles, layout); setTileState((state) => ({ ...state, tiles: newTiles })); } From 17fed7cd9cc311db3888c5a3541ce1c0ce18c838 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Tue, 24 May 2022 16:55:53 -0400 Subject: [PATCH 3/3] Prettyify --- src/video-grid/VideoGrid.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video-grid/VideoGrid.jsx b/src/video-grid/VideoGrid.jsx index a0db691..2ae7cbd 100644 --- a/src/video-grid/VideoGrid.jsx +++ b/src/video-grid/VideoGrid.jsx @@ -606,7 +606,7 @@ function getSubGridPositions( function reorderTiles(tiles, layout) { if (layout === "freedom" && tiles.length === 2) { // 1:1 layout - tiles.forEach((tile) => tile.order = tile.item.isLocal ? 0 : 1); + tiles.forEach((tile) => (tile.order = tile.item.isLocal ? 0 : 1)); } else { const focusedTiles = []; const presenterTiles = [];