From bf4e697a0a77b07848f6ded998b727ca51652c16 Mon Sep 17 00:00:00 2001 From: Alvaro <110414366+alvaro-signal@users.noreply.github.com> Date: Mon, 17 Oct 2022 10:58:49 -0600 Subject: [PATCH] Fixed story replies modal and calling pip interactions --- stylesheets/_modules.scss | 16 ++++++++-- ts/components/CallingParticipantsList.tsx | 11 ++++--- ts/components/CallingPip.tsx | 2 +- ts/components/ModalContainer.tsx | 30 +++++++++++++++++++ ts/components/ModalHost.tsx | 36 ++++++++++++++++------- ts/components/StoryViewer.tsx | 2 +- ts/state/smart/App.tsx | 7 ++++- ts/util/lint/exceptions.json | 7 +++++ 8 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 ts/components/ModalContainer.tsx diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index 1fc6f4a77be..0f950dd7076 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -3634,6 +3634,18 @@ button.module-image__border-overlay:focus { // Module: Calling .module-calling { + // creates a new independent stacking context that includes modals + // + // container has no width/height, direct children need to: + // - size themselves explicitly (no percentage width/height or top/bottom or left/right) + // - size themselves in relation to viewport (position: fixed) + &__modal-container { + position: fixed; + top: 0; + left: 0; + z-index: $z-index-on-top-of-everything; + } + &__container { align-items: center; background-color: $calling-background-color; @@ -3641,7 +3653,7 @@ button.module-image__border-overlay:focus { flex-direction: column; height: var(--window-height); justify-content: center; - position: absolute; + position: fixed; width: 100%; z-index: $z-index-calling; } @@ -4097,7 +4109,7 @@ button.module-image__border-overlay:focus { box-shadow: 0px 0px 8px rgba(0, 0, 0, 0.05), 0px 8px 20px rgba(0, 0, 0, 0.3); cursor: grab; height: 158px; - position: absolute; + position: fixed; width: 120px; z-index: $z-index-calling-pip; diff --git a/ts/components/CallingParticipantsList.tsx b/ts/components/CallingParticipantsList.tsx index 0a2a2221762..1156e1f2d3e 100644 --- a/ts/components/CallingParticipantsList.tsx +++ b/ts/components/CallingParticipantsList.tsx @@ -3,7 +3,7 @@ /* eslint-disable react/no-array-index-key */ -import React from 'react'; +import React, { useContext } from 'react'; import { createPortal } from 'react-dom'; import FocusTrap from 'focus-trap-react'; @@ -14,6 +14,7 @@ import type { LocalizerType } from '../types/Util'; import { sortByTitle } from '../util/sortByTitle'; import type { ConversationType } from '../state/ducks/conversations'; import { isInSystemContacts } from '../util/isInSystemContacts'; +import { ModalContainerContext } from './ModalHost'; type ParticipantType = ConversationType & { hasRemoteAudio?: boolean; @@ -32,6 +33,8 @@ export const CallingParticipantsList = React.memo( ({ i18n, onClose, ourUuid, participants }: PropsType) => { const [root, setRoot] = React.useState(null); + const modalContainer = useContext(ModalContainerContext) ?? document.body; + const sortedParticipants = React.useMemo>( () => sortByTitle(participants), [participants] @@ -39,14 +42,14 @@ export const CallingParticipantsList = React.memo( React.useEffect(() => { const div = document.createElement('div'); - document.body.appendChild(div); + modalContainer.appendChild(div); setRoot(div); return () => { - document.body.removeChild(div); + modalContainer.removeChild(div); setRoot(null); }; - }, []); + }, [modalContainer]); const handleCancel = React.useCallback( (e: React.MouseEvent) => { diff --git a/ts/components/CallingPip.tsx b/ts/components/CallingPip.tsx index 5112f3062d1..2d934a0bab1 100644 --- a/ts/components/CallingPip.tsx +++ b/ts/components/CallingPip.tsx @@ -82,7 +82,7 @@ export const CallingPip = ({ switchToPresentationView, switchFromPresentationView, togglePip, -}: PropsType): JSX.Element | null => { +}: PropsType): JSX.Element => { const videoContainerRef = React.useRef(null); const localVideoRef = React.useRef(null); diff --git a/ts/components/ModalContainer.tsx b/ts/components/ModalContainer.tsx new file mode 100644 index 00000000000..b5adf1e9d1a --- /dev/null +++ b/ts/components/ModalContainer.tsx @@ -0,0 +1,30 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import React from 'react'; +import type { ReactNode } from 'react'; +import ReactDOM from 'react-dom'; +import { ModalContainerContext } from './ModalHost'; + +type Props = { + children: ReactNode; + className?: string; +}; + +/** + * Provide a div directly under the document.body that Modals can use as a DOM parent. + * + * Useful when you want to control the stacking context of all children, by customizing + * the styles of the container in way that also applies to modals. + */ +export const ModalContainer = ({ children, className }: Props): JSX.Element => { + const containerRef = React.useRef(null); + return ReactDOM.createPortal( +
+ + {children} + +
, + document.body + ); +}; diff --git a/ts/components/ModalHost.tsx b/ts/components/ModalHost.tsx index 483c5e54e19..55d736dd302 100644 --- a/ts/components/ModalHost.tsx +++ b/ts/components/ModalHost.tsx @@ -1,7 +1,7 @@ // Copyright 2019-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import React, { useEffect } from 'react'; +import React, { useContext, useEffect } from 'react'; import { createPortal } from 'react-dom'; import FocusTrap from 'focus-trap-react'; import type { SpringValues } from '@react-spring/web'; @@ -19,6 +19,10 @@ import { usePrevious } from '../hooks/usePrevious'; import { handleOutsideClick } from '../util/handleOutsideClick'; import * as log from '../logging/log'; +export const ModalContainerContext = React.createContext( + null +); + export type PropsType = Readonly<{ children: React.ReactElement; modalName: string; @@ -48,6 +52,7 @@ export const ModalHost = React.memo( const [root, setRoot] = React.useState(null); const containerRef = React.useRef(null); const previousModalName = usePrevious(modalName, modalName); + const modalContainer = useContext(ModalContainerContext) ?? document.body; if (previousModalName !== modalName) { log.error( @@ -59,14 +64,14 @@ export const ModalHost = React.memo( useEffect(() => { const div = document.createElement('div'); - document.body.appendChild(div); + modalContainer.appendChild(div); setRoot(div); return () => { - document.body.removeChild(div); + modalContainer.removeChild(div); setRoot(null); }; - }, []); + }, [modalContainer]); useEscapeHandling(onEscape || onClose); useEffect(() => { @@ -74,13 +79,22 @@ export const ModalHost = React.memo( return noop; } return handleOutsideClick( - () => { + node => { + // ignore clicks that originate in the calling/pip + // when we're not handling a component in the calling/pip + if ( + modalContainer === document.body && + node instanceof Element && + node.closest('.module-calling__modal-container') + ) { + return false; + } onClose(); return true; }, { containerElements: [containerRef], name: modalName } ); - }, [noMouseClose, onClose, containerRef, modalName]); + }, [noMouseClose, onClose, containerRef, modalName, modalContainer]); const className = classNames([ theme ? themeClassName(theme) : undefined, @@ -113,12 +127,14 @@ export const ModalHost = React.memo( return false; } - // TitleBar should always receive clicks. Quill suggestions - // are placed in the document.body so they should be exempt - // too. + // Exemptions: + // - TitleBar should always receive clicks. + // - Quill suggestions since they are placed in the document.body + // - Calling module (and pip) are always above everything else const exemptParent = target.closest( '.TitleBarContainer__title, ' + - '.module-composition-input__suggestions' + '.module-composition-input__suggestions, ' + + '.module-calling__modal-container' ); if (exemptParent) { return true; diff --git a/ts/components/StoryViewer.tsx b/ts/components/StoryViewer.tsx index 37bbb488700..1cc011d166c 100644 --- a/ts/components/StoryViewer.tsx +++ b/ts/components/StoryViewer.tsx @@ -487,7 +487,7 @@ export const StoryViewer = ({ ]; return ( - +
{ const i18n = getIntl(state); @@ -44,7 +45,11 @@ const mapStateToProps = (state: StateType) => { menuOptions: getMenuOptions(state), hasCustomTitleBar: window.SignalContext.OS.hasCustomTitleBar(), hideMenuBar: getHideMenuBar(state), - renderCallManager: () => , + renderCallManager: () => ( + + + + ), renderCustomizingPreferredReactionsModal: () => ( ), diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 1f2601169bc..9abcab33c9b 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -22,6 +22,13 @@ "reasonCategory": "falseMatch", "updated": "2018-09-19T18:13:29.628Z" }, + { + "rule": "React-useRef", + "path": "ts/components/ModalContainer.tsx", + "line": " const containerRef = React.useRef(null);", + "reasonCategory": "usageTrusted", + "updated": "2022-10-14T16:39:48.461Z" + }, { "rule": "jQuery-globalEval(", "path": "components/mp3lameencoder/lib/Mp3LameEncoder.js",