Make PiP motion smoother and react to window resizes correctly (#8747)

* Make PiP motion smoother and react to window resizes correctly

* Remove debugging logs

* Apply code review suggestions

src/components/views/elements/AppTile.tsx

@@ -18,7 +18,7 @@ limitations under the License.
 */
 
 import url from 'url';
-import React, { ContextType, createRef } from 'react';
+import React, { ContextType, createRef, MutableRefObject } from 'react';
 import classNames from 'classnames';
 import { MatrixCapabilities } from "matrix-widget-api";
 import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
@@ -84,6 +84,8 @@ interface IProps {
     pointerEvents?: string;
     widgetPageTitle?: string;
     showLayoutButtons?: boolean;
+    // Handle to manually notify the PersistedElement that it needs to move
+    movePersistedElement?: MutableRefObject<() => void>;
 }
 
 interface IState {
@@ -623,7 +625,11 @@ export default class AppTile extends React.Component<IProps, IState> {
                     const zIndexAboveOtherPersistentElements = 101;
 
                     appTileBody = <div className="mx_AppTile_persistedWrapper">
-                        <PersistedElement zIndex={this.props.miniMode ? zIndexAboveOtherPersistentElements : 9} persistKey={this.persistKey}>
+                        <PersistedElement
+                            zIndex={this.props.miniMode ? zIndexAboveOtherPersistentElements : 9}
+                            persistKey={this.persistKey}
+                            moveRef={this.props.movePersistedElement}
+                        >
                             { appTileBody }
                         </PersistedElement>
                     </div>;

src/components/views/elements/PersistedElement.tsx

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React from 'react';
+import React, { MutableRefObject } from 'react';
 import ReactDOM from 'react-dom';
 import { throttle } from "lodash";
 import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
@@ -56,6 +56,9 @@ interface IProps {
     zIndex?: number;
 
     style?: React.StyleHTMLAttributes<HTMLDivElement>;
+
+    // Handle to manually notify this PersistedElement that it needs to move
+    moveRef?: MutableRefObject<() => void>;
 }
 
 /**
@@ -86,6 +89,8 @@ export default class PersistedElement extends React.Component<IProps> {
         // the timeline_resize action.
         window.addEventListener('resize', this.repositionChild);
         this.dispatcherRef = dis.register(this.onAction);
+
+        if (this.props.moveRef) this.props.moveRef.current = this.repositionChild;
     }
 
     /**
@@ -177,8 +182,9 @@ export default class PersistedElement extends React.Component<IProps> {
         Object.assign(child.style, {
             zIndex: isNullOrUndefined(this.props.zIndex) ? 9 : this.props.zIndex,
             position: 'absolute',
-            top: parentRect.top + 'px',
+            top: '0',
-            left: parentRect.left + 'px',
+            left: '0',
+            transform: `translateX(${parentRect.left}px) translateY(${parentRect.top}px)`,
             width: parentRect.width + 'px',
             height: parentRect.height + 'px',
         });

src/components/views/elements/PersistentApp.tsx

@@ -1,6 +1,6 @@
 /*
 Copyright 2018 New Vector Ltd
-Copyright 2019, 2020 The Matrix.org Foundation C.I.C.
+Copyright 2019-2022 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { ContextType } from 'react';
+import React, { ContextType, MutableRefObject } from 'react';
 import { Room } from "matrix-js-sdk/src/models/room";
 
 import WidgetUtils from '../../../utils/WidgetUtils';
@@ -27,6 +27,7 @@ interface IProps {
     persistentWidgetId: string;
     persistentRoomId: string;
     pointerEvents?: string;
+    movePersistedElement: MutableRefObject<() => void>;
 }
 
 export default class PersistentApp extends React.Component<IProps> {
@@ -70,6 +71,7 @@ export default class PersistentApp extends React.Component<IProps> {
                 miniMode={true}
                 showMenubar={false}
                 pointerEvents={this.props.pointerEvents}
+                movePersistedElement={this.props.movePersistedElement}
             />;
         }
         return null;

src/components/views/voip/PictureInPictureDragger.tsx

@@ -1,5 +1,5 @@
 /*
-Copyright 2021 New Vector Ltd
+Copyright 2021-2022 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -16,7 +16,7 @@ limitations under the License.
 
 import React, { createRef } from 'react';
 
-import UIStore from '../../../stores/UIStore';
+import UIStore, { UI_EVENTS } from '../../../stores/UIStore';
 import { lerp } from '../../../utils/AnimationUtils';
 import { MarkedExecution } from '../../../utils/MarkedExecution';
 
@@ -43,69 +43,66 @@ interface IProps {
     children: ({ onStartMoving, onResize }: IChildrenOptions) => React.ReactNode;
     draggable: boolean;
     onDoubleClick?: () => void;
-}
+    onMove?: () => void;
-
-interface IState {
-    // Position of the PictureInPictureDragger
-    translationX: number;
-    translationY: number;
 }
 
 /**
  * PictureInPictureDragger shows a small version of CallView hovering over the UI in 'picture-in-picture'
  * (PiP mode). It displays the call(s) which is *not* in the room the user is currently viewing.
  */
-export default class PictureInPictureDragger extends React.Component<IProps, IState> {
+export default class PictureInPictureDragger extends React.Component<IProps> {
     private callViewWrapper = createRef<HTMLDivElement>();
     private initX = 0;
     private initY = 0;
     private desiredTranslationX = UIStore.instance.windowWidth - PADDING.right - PIP_VIEW_WIDTH;
     private desiredTranslationY = UIStore.instance.windowHeight - PADDING.bottom - PIP_VIEW_HEIGHT;
+    private translationX = this.desiredTranslationX;
+    private translationY = this.desiredTranslationY;
     private moving = false;
     private scheduledUpdate = new MarkedExecution(
         () => this.animationCallback(),
         () => requestAnimationFrame(() => this.scheduledUpdate.trigger()),
     );
 
-    constructor(props: IProps) {
-        super(props);
-
-        this.state = {
-            translationX: UIStore.instance.windowWidth - PADDING.right - PIP_VIEW_WIDTH,
-            translationY: UIStore.instance.windowHeight - PADDING.bottom - PIP_VIEW_HEIGHT,
-        };
-    }
-
     public componentDidMount() {
         document.addEventListener("mousemove", this.onMoving);
         document.addEventListener("mouseup", this.onEndMoving);
-        window.addEventListener("resize", this.onResize);
+        UIStore.instance.on(UI_EVENTS.Resize, this.onResize);
     }
 
     public componentWillUnmount() {
         document.removeEventListener("mousemove", this.onMoving);
         document.removeEventListener("mouseup", this.onEndMoving);
-        window.removeEventListener("resize", this.onResize);
+        UIStore.instance.off(UI_EVENTS.Resize, this.onResize);
     }
 
     private animationCallback = () => {
-        // If the PiP isn't being dragged and there is only a tiny difference in
-        // the desiredTranslation and translation, quit the animationCallback
-        // loop. If that is the case, it means the PiP has snapped into its
-        // position and there is nothing to do. Not doing this would cause an
-        // infinite loop
         if (
             !this.moving &&
-            Math.abs(this.state.translationX - this.desiredTranslationX) <= 1 &&
+            Math.abs(this.translationX - this.desiredTranslationX) <= 1 &&
-            Math.abs(this.state.translationY - this.desiredTranslationY) <= 1
+            Math.abs(this.translationY - this.desiredTranslationY) <= 1
-        ) return;
+        ) {
+            // Break the loop by settling the element into its final position
+            this.translationX = this.desiredTranslationX;
+            this.translationY = this.desiredTranslationY;
+            this.setStyle();
+        } else {
+            const amt = this.moving ? MOVING_AMT : SNAPPING_AMT;
+            this.translationX = lerp(this.translationX, this.desiredTranslationX, amt);
+            this.translationY = lerp(this.translationY, this.desiredTranslationY, amt);
 
-        const amt = this.moving ? MOVING_AMT : SNAPPING_AMT;
+            this.setStyle();
-        this.setState({
+            this.scheduledUpdate.mark();
-            translationX: lerp(this.state.translationX, this.desiredTranslationX, amt),
+        }
-            translationY: lerp(this.state.translationY, this.desiredTranslationY, amt),
+
-        });
+        this.props.onMove?.();
-        this.scheduledUpdate.mark();
+    };
+
+    private setStyle = () => {
+        if (!this.callViewWrapper.current) return;
+        // Set the element's style directly, bypassing React for efficiency
+        this.callViewWrapper.current.style.transform =
+            `translateX(${this.translationX}px) translateY(${this.translationY}px)`;
     };
 
     private setTranslation(inTranslationX: number, inTranslationY: number) {
@@ -164,20 +161,14 @@ export default class PictureInPictureDragger extends React.Component<IProps, ISt
             this.desiredTranslationY = PADDING.top;
         }
 
+        if (!animate) {
+            this.translationX = this.desiredTranslationX;
+            this.translationY = this.desiredTranslationY;
+        }
+
         // We start animating here because we want the PiP to move when we're
         // resizing the window
         this.scheduledUpdate.mark();
-
-        if (animate) {
-            // We start animating here because we want the PiP to move when we're
-            // resizing the window
-            this.scheduledUpdate.mark();
-        } else {
-            this.setState({
-                translationX: this.desiredTranslationX,
-                translationY: this.desiredTranslationY,
-            });
-        }
     };
 
     private onStartMoving = (event: React.MouseEvent | MouseEvent) => {
@@ -205,25 +196,21 @@ export default class PictureInPictureDragger extends React.Component<IProps, ISt
     };
 
     public render() {
-        const translatePixelsX = this.state.translationX + "px";
-        const translatePixelsY = this.state.translationY + "px";
         const style = {
-            transform: `translateX(${translatePixelsX})
+            transform: `translateX(${this.translationX}px) translateY(${this.translationY}px)`,
-                        translateY(${translatePixelsY})`,
         };
+
         return (
             <div
                 className={this.props.className}
-                style={this.props.draggable ? style : undefined}
+                style={style}
                 ref={this.callViewWrapper}
                 onDoubleClick={this.props.onDoubleClick}
             >
-                <>
+                { this.props.children({
-                    { this.props.children({
+                    onStartMoving: this.onStartMoving,
-                        onStartMoving: this.onStartMoving,
+                    onResize: this.onResize,
-                        onResize: this.onResize,
+                }) }
-                    }) }
-                </>
             </div>
         );
     }

src/components/views/voip/PipView.tsx

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React from 'react';
+import React, { createRef } from 'react';
 import { CallEvent, CallState, MatrixCall } from 'matrix-js-sdk/src/webrtc/call';
 import { EventSubscription } from 'fbemitter';
 import { logger } from "matrix-js-sdk/src/logger";
@@ -118,6 +118,7 @@ function getPrimarySecondaryCallsForPip(roomId: string): [MatrixCall, MatrixCall
 export default class PipView extends React.Component<IProps, IState> {
     private roomStoreToken: EventSubscription;
     private settingsWatcherRef: string;
+    private movePersistedElement = createRef<() => void>();
 
     constructor(props: IProps) {
         super(props);
@@ -176,6 +177,8 @@ export default class PipView extends React.Component<IProps, IState> {
         this.setState({ moving: false });
     }
 
+    private onMove = () => this.movePersistedElement.current?.();
+
     private onRoomViewStoreUpdate = () => {
         const newRoomId = RoomViewStore.instance.getRoomId();
         const oldRoomId = this.state.viewedRoomId;
@@ -338,6 +341,7 @@ export default class PipView extends React.Component<IProps, IState> {
                         persistentWidgetId={this.state.persistentWidgetId}
                         persistentRoomId={roomId}
                         pointerEvents={this.state.moving ? 'none' : undefined}
+                        movePersistedElement={this.movePersistedElement}
                     />
                 </div>;
         }
@@ -347,6 +351,7 @@ export default class PipView extends React.Component<IProps, IState> {
                 className="mx_CallPreview"
                 draggable={pipMode}
                 onDoubleClick={this.onDoubleClick}
+                onMove={this.onMove}
             >
                 { pipContent }
             </PictureInPictureDragger>;