From d71d72024565718f17d5ea5a7ff8d52fa9fd350b Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 9 Sep 2026 14:07:12 -0400 Subject: [PATCH] refactor(common-adapters): give each popup mode its own module Popup was one 255-line dispatcher behind a 16-field interface that branched on runtime state into four unrelated presentations, each reading a different subset of the props and silently ignoring the rest. The platform rule was encoded independently in three places. Split the presentations into three modules with narrow interfaces - AnchoredPopup, Sheet, ModalCover - and keep Popup as a policy module that picks one from an explicit intent ('menu' or 'dialog'). The platform default now lives only there, so usePopup2 no longer gates the anchor ref and min-writer-role no longer gates its ref either. The type system rejects placement props on a mode that ignores them, and hideKeyboard now exists only on the mode that implements it. Popup no longer takes visible: every caller passed a literal true. FloatingMenu was the exception - it let mode='modal' past its own visibility guard and leaned on Popup to drop the hidden menu on the way past, so that guard is now unconditional. Sheet and ModalCover get file suffixes, so @gorhom/bottom-sheet and react-native-screens leave the desktop bundle and the raw div leaves the native one. ModalCover, previously untestable inside the dispatcher, gets tests for its press and escape handling. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015rccpV5nLxxC5opF5xzrz7 --- .../info-panel/settings/min-writer-role.tsx | 2 +- .../conversation/input-area/normal/input.tsx | 2 +- .../input-area/suggestors/index.test.tsx | 2 +- .../input-area/suggestors/index.tsx | 6 +- .../chat/conversation/messages/emoji-row.tsx | 1 + .../messages/reaction-tooltip.tsx | 8 +- .../conversation-list/choose-conversation.tsx | 2 +- shared/common-adapters/dropdown.tsx | 2 +- .../floating-menu/index.test.tsx | 26 ++ .../common-adapters/floating-menu/index.tsx | 8 +- shared/common-adapters/floating-picker.tsx | 11 +- shared/common-adapters/index.tsx | 4 + shared/common-adapters/popup/anchored.tsx | 56 ++++ shared/common-adapters/popup/bottom-sheet.tsx | 72 ----- shared/common-adapters/popup/index.shared.tsx | 55 +++- shared/common-adapters/popup/index.tsx | 276 +++--------------- .../popup/modal-cover.desktop.tsx | 78 +++++ .../popup/modal-cover.test.tsx | 92 ++++++ shared/common-adapters/popup/modal-cover.tsx | 7 + shared/common-adapters/popup/sheet.native.tsx | 127 ++++++++ shared/common-adapters/popup/sheet.tsx | 7 + .../common-adapters/popup/use-popup.test.tsx | 6 +- shared/common-adapters/popup/use-popup.tsx | 5 +- shared/common-adapters/toast.tsx | 14 +- shared/fs/common/sfmi-popup.tsx | 8 +- shared/login/reset/modal.tsx | 5 +- shared/provision/code-page/container.tsx | 2 +- shared/teams/common/enable-contacts.tsx | 2 +- shared/teams/common/selection-popup.tsx | 2 +- shared/teams/common/use-autocompleter.tsx | 1 + shared/teams/common/use-team-link-popup.tsx | 8 +- shared/teams/emojis/add-alias.tsx | 1 + shared/teams/role-picker.tsx | 5 +- .../teams/team/settings-tab/channel-popup.tsx | 2 +- shared/tsconfig.native.json | 2 + 35 files changed, 540 insertions(+), 367 deletions(-) create mode 100644 shared/common-adapters/floating-menu/index.test.tsx create mode 100644 shared/common-adapters/popup/anchored.tsx delete mode 100644 shared/common-adapters/popup/bottom-sheet.tsx create mode 100644 shared/common-adapters/popup/modal-cover.desktop.tsx create mode 100644 shared/common-adapters/popup/modal-cover.test.tsx create mode 100644 shared/common-adapters/popup/modal-cover.tsx create mode 100644 shared/common-adapters/popup/sheet.native.tsx create mode 100644 shared/common-adapters/popup/sheet.tsx diff --git a/shared/chat/conversation/info-panel/settings/min-writer-role.tsx b/shared/chat/conversation/info-panel/settings/min-writer-role.tsx index 723d0b3f515a..fc1dbb8b99bf 100644 --- a/shared/chat/conversation/info-panel/settings/min-writer-role.tsx +++ b/shared/chat/conversation/info-panel/settings/min-writer-role.tsx @@ -140,7 +140,7 @@ const Dropdown = (p: DropdownProps) => { direction="horizontal" alignItems="center" style={styles.dropdown} - ref={isMobile ? null : popupAnchor} + ref={popupAnchor} onClick={showPopup} > diff --git a/shared/chat/conversation/input-area/normal/input.tsx b/shared/chat/conversation/input-area/normal/input.tsx index 285eda1e0f6f..e89e044e3a45 100644 --- a/shared/chat/conversation/input-area/normal/input.tsx +++ b/shared/chat/conversation/input-area/normal/input.tsx @@ -548,7 +548,7 @@ const EmojiButton = function EmojiButton(p: EmojiButtonProps) { const makePopup = (p: Kb.Popup2Parms) => { const {attachTo, hidePopup} = p return ( - + ({useConversationThreadID: () => 'conv'} jest.mock('../input-state', () => ({useConversationInput: () => false})) jest.mock('@/common-adapters', () => { const actual = jest.requireActual>('@/common-adapters') - return {...actual, Popup: (p: {children: React.ReactNode}) => <>{p.children}} + return {...actual, AnchoredPopup: (p: {children: React.ReactNode}) => <>{p.children}} }) // the suggestors read the caret through the input ref; drive it directly so the diff --git a/shared/chat/conversation/input-area/suggestors/index.tsx b/shared/chat/conversation/input-area/suggestors/index.tsx index 670e6e72ceb7..130f4a32bad4 100644 --- a/shared/chat/conversation/input-area/suggestors/index.tsx +++ b/shared/chat/conversation/input-area/suggestors/index.tsx @@ -479,20 +479,18 @@ const Popup = (p: PopupProps) => { const attachRef = inputRef as React.RefObject return ( - {isMobile ? {children} : children} - + ) } diff --git a/shared/chat/conversation/messages/emoji-row.tsx b/shared/chat/conversation/messages/emoji-row.tsx index 101df01d04f2..bda9097be479 100644 --- a/shared/chat/conversation/messages/emoji-row.tsx +++ b/shared/chat/conversation/messages/emoji-row.tsx @@ -193,6 +193,7 @@ function EmojiRowContainer(p: OwnProps) { {showingPicker && message && hasMessageID && ( { if (isMobile) { return ( - { ))} - + ) } return ( - { /> - + ) } diff --git a/shared/chat/send-to-chat/conversation-list/choose-conversation.tsx b/shared/chat/send-to-chat/conversation-list/choose-conversation.tsx index 02d84aec3674..4112efeff894 100644 --- a/shared/chat/send-to-chat/conversation-list/choose-conversation.tsx +++ b/shared/chat/send-to-chat/conversation-list/choose-conversation.tsx @@ -17,11 +17,11 @@ const ChooseConversation = (props: Props) => { const {attachTo, hidePopup} = p return ( diff --git a/shared/common-adapters/dropdown.tsx b/shared/common-adapters/dropdown.tsx index 0579ebf4840d..a5022ef785ba 100644 --- a/shared/common-adapters/dropdown.tsx +++ b/shared/common-adapters/dropdown.tsx @@ -115,9 +115,9 @@ function Dropdown(p: Props) { )) return ( diff --git a/shared/common-adapters/floating-menu/index.test.tsx b/shared/common-adapters/floating-menu/index.test.tsx new file mode 100644 index 000000000000..8f54b7fbad78 --- /dev/null +++ b/shared/common-adapters/floating-menu/index.test.tsx @@ -0,0 +1,26 @@ +/** @jest-environment jsdom */ +/// + +import {cleanup, render} from '@testing-library/react' +import FloatingMenu from '.' + +const items = [{onClick: () => {}, title: 'an item'}] + +describe('FloatingMenu visibility', () => { + afterEach(() => { + cleanup() + }) + + // 'modal' used to skip this guard and rely on Popup dropping an invisible + // popup on the way past. Popup no longer takes visible, so the guard is the + // only thing keeping a hidden menu off the screen. + test.each([undefined, 'bottomsheet', 'modal'] as const)( + 'renders nothing when hidden in %s mode', + mode => { + const {container} = render( + {}} visible={false} /> + ) + expect(container.innerHTML).toBe('') + } + ) +}) diff --git a/shared/common-adapters/floating-menu/index.tsx b/shared/common-adapters/floating-menu/index.tsx index a0141876b704..8045b9577ba2 100644 --- a/shared/common-adapters/floating-menu/index.tsx +++ b/shared/common-adapters/floating-menu/index.tsx @@ -50,9 +50,9 @@ function FloatingMenu(props: Props) { return unsub }, [navigation, onHidden]) - // modal mode callers control mounting themselves; sheets present on mount so - // they must unmount when not visible - if (!visible && mode !== 'modal') { + // sheets present on mount, so an invisible menu must not render at all. Popup + // used to catch the modal case on the way past; it no longer takes visible. + if (!visible) { return null } @@ -77,9 +77,9 @@ function FloatingMenu(props: Props) { return ( = { @@ -24,7 +24,7 @@ export type Props = { visible: boolean } -const Kb = {Box2, Picker, Popup, SafeAreaView, Text} +const Kb = {Box2, Picker, SafeAreaView, Sheet, Text} function WrapPicker(p: { initialValue?: T @@ -68,10 +68,7 @@ const FloatingPicker = (props: Props): React.React } return ( - + {props.header} (props: Props): React.React /> - + ) } diff --git a/shared/common-adapters/index.tsx b/shared/common-adapters/index.tsx index 1f4746683e54..9b9851df5cab 100644 --- a/shared/common-adapters/index.tsx +++ b/shared/common-adapters/index.tsx @@ -74,6 +74,10 @@ export {default as Markdown} from './markdown' export {default as Meta} from './meta' export {default as NameWithIcon} from './name-with-icon' export {default as Popup, type PopupProps} from './popup/index' +export {AnchoredPopup, type AnchoredPopupProps} from './popup/anchored' +export {ModalCover, type ModalCoverProps} from './popup/modal-cover' +export {Sheet, type SheetProps} from './popup/sheet' +export {Portal} from './portal' export {default as PhoneInput} from './phone-input' export {default as Placeholder} from './placeholder' export {default as ProgressBar} from './progress-bar' diff --git a/shared/common-adapters/popup/anchored.tsx b/shared/common-adapters/popup/anchored.tsx new file mode 100644 index 000000000000..b70426a15f3d --- /dev/null +++ b/shared/common-adapters/popup/anchored.tsx @@ -0,0 +1,56 @@ +import * as Styles from '@/styles' +import {Box2} from '../box' +import FloatingBox from './floating-box' +import type {AnchoredPopupProps} from './index.shared' +export type {AnchoredPopupProps} from './index.shared' + +export const AnchoredPopup = (props: AnchoredPopupProps) => { + const {attachTo, children, containerStyle, hideKeyboard, matchDimension, offset, onHidden} = props + const {position, positionFallbacks, propagateOutsideClicks, remeasureHint, style} = props + const styles = useStyles() + + if (isMobile) { + // on mobile FloatingBox is the portal + keyboard-dismiss overlay this needs, + // and there is no positioner, so the anchor and placement props go unused + return ( + + {children} + + ) + } + + return ( + + {onHidden ? ( + + {children} + + ) : ( + children + )} + + ) +} + +const useStyles = Styles.createStyleHook(() => ({ + positioned: Styles.platformStyles({ + isElectron: { + ...Styles.desktopStyles.boxShadow, + ...Styles.globalStyles.rounded, + overflowX: 'hidden', + overflowY: 'auto', + }, + }), +})) + +export default AnchoredPopup diff --git a/shared/common-adapters/popup/bottom-sheet.tsx b/shared/common-adapters/popup/bottom-sheet.tsx deleted file mode 100644 index 135899218cad..000000000000 --- a/shared/common-adapters/popup/bottom-sheet.tsx +++ /dev/null @@ -1,72 +0,0 @@ -import * as React from 'react' -import type {StylesCrossPlatform} from '@/styles' -import type {BottomSheetModalProps, BottomSheetBackdropProps, BottomSheetFooterProps} from '@gorhom/bottom-sheet' -import * as _gorhomRaw from '@gorhom/bottom-sheet' - -type NativeMethods = {present: () => void; forceClose: () => void} -type BackdropProps = BottomSheetBackdropProps & {disappearsOnIndex?: number; appearsOnIndex?: number; opacity?: number} -type ScrollViewProps = { - style?: StylesCrossPlatform - contentContainerStyle?: StylesCrossPlatform - automaticallyAdjustsScrollIndicatorInsets?: boolean - contentInsetAdjustmentBehavior?: 'automatic' | 'scrollableAxes' | 'never' | 'always' - scrollIndicatorInsets?: {top?: number; left?: number; bottom?: number; right?: number} - children?: React.ReactNode - enableFooterMarginAdjustment?: boolean - alwaysBounceVertical?: boolean - overScrollMode?: 'auto' | 'always' | 'never' -} -type FooterProps = BottomSheetFooterProps & {bottomInset?: number; children?: React.ReactNode} -type GorhomModule = { - BottomSheetModal: React.ForwardRefExoticComponent> - BottomSheetBackdrop: React.ComponentType - BottomSheetScrollView: React.ComponentType - BottomSheetFooter: React.ComponentType -} - -const _gorhom: GorhomModule | null = isMobile ? (_gorhomRaw as unknown as GorhomModule) : null - -export class BottomSheetModal extends React.Component { - private _native: NativeMethods | null = null - - present() { - this._native?.present() - } - - forceClose() { - this._native?.forceClose() - } - - override render() { - if (!isMobile) return null - const {BottomSheetModal: NativeModal} = _gorhom! - return ( - { - this._native = r - }} - /> - ) - } -} - -export const BottomSheetBackdrop = (_p: BackdropProps) => { - if (!isMobile) return null - const {BottomSheetBackdrop: NativeBackdrop} = _gorhom! - return -} - -export const BottomSheetScrollView = (_p: ScrollViewProps) => { - if (!isMobile) return null - const {BottomSheetScrollView: NativeScrollView} = _gorhom! - return -} - -export const BottomSheetFooter = (_p: FooterProps) => { - if (!isMobile) return null - const {BottomSheetFooter: NativeFooter} = _gorhom! - return -} - -export type {BottomSheetBackdropProps, BottomSheetFooterProps} from '@gorhom/bottom-sheet' diff --git a/shared/common-adapters/popup/index.shared.tsx b/shared/common-adapters/popup/index.shared.tsx index 1ee59af203d8..0cf6ac92620e 100644 --- a/shared/common-adapters/popup/index.shared.tsx +++ b/shared/common-adapters/popup/index.shared.tsx @@ -2,24 +2,65 @@ import type {MeasureRef} from '@/common-adapters/measure-ref' import type * as React from 'react' import type * as Styles from '@/styles' -export type PopupProps = { +// Anchored to an on-screen element. Desktop measures the anchor and positions +// against it; mobile has no positioner and only covers the screen, so it reads +// none of the placement props. +export type AnchoredPopupProps = { children: React.ReactNode onHidden?: () => void attachTo?: React.RefObject - // mobile ignores attachTo and presents a bottom sheet by default; set this - // to keep the raw anchored portal on mobile (e.g. positioned over an input) - mobileAnchored?: boolean position?: Styles.Position positionFallbacks?: ReadonlyArray propagateOutsideClicks?: boolean matchDimension?: boolean remeasureHint?: number offset?: number - style?: Styles.StylesCrossPlatform containerStyle?: Styles.StylesCrossPlatform - visible?: boolean + // desktop only: styles the shadowed box drawn around the content, which only + // exists when onHidden is set. mobile has no such box - use containerStyle. + style?: Styles.StylesCrossPlatform + // mobile only hideKeyboard?: boolean +} + +// Mobile bottom sheet. Presents on mount, so callers mount it only while shown. +export type SheetProps = { + children: React.ReactNode + onHidden: () => void + snapPoints?: Array + // pinned below the scrolling content, always visible + footer?: React.ReactNode + style?: Styles.StylesCrossPlatform +} + +// Desktop full-window cover with the content centered on it. +export type ModalCoverProps = { + children: React.ReactNode + onHidden?: () => void + style?: Styles.StylesCrossPlatform +} + +type SharedPopupProps = { + children: React.ReactNode + onHidden: () => void + style?: Styles.StylesCrossPlatform + // mobile sheet only snapPoints?: Array - // mobile sheet only: pinned below the scrolling content, always visible footer?: React.ReactNode } + +export type PopupProps = SharedPopupProps & + ( + | ({intent: 'menu'} & Pick< + AnchoredPopupProps, + | 'attachTo' + | 'containerStyle' + | 'matchDimension' + | 'offset' + | 'position' + | 'positionFallbacks' + | 'propagateOutsideClicks' + | 'remeasureHint' + >) + | {intent: 'dialog'} + ) diff --git a/shared/common-adapters/popup/index.tsx b/shared/common-adapters/popup/index.tsx index dce8858115ea..568c4dcf0475 100644 --- a/shared/common-adapters/popup/index.tsx +++ b/shared/common-adapters/popup/index.tsx @@ -1,255 +1,47 @@ -import * as React from 'react' -import * as Styles from '@/styles' -import {Box2} from '../box' -import FloatingBox from './floating-box' -import {EscapeHandler} from '../key-event-handler' -import {Portal} from '../portal' -import { - BottomSheetModal, - BottomSheetScrollView, - BottomSheetBackdrop, - BottomSheetFooter, - type BottomSheetBackdropProps, - type BottomSheetFooterProps, -} from './bottom-sheet' -import {useSafeAreaInsets} from '../safe-area-view' -import {initialWindowMetrics} from 'react-native-safe-area-context' -import {FullWindowOverlay} from 'react-native-screens' -import {Keyboard} from 'react-native' +import {AnchoredPopup} from './anchored' +import {ModalCover} from './modal-cover' +import {Sheet} from './sheet' import type {PopupProps} from './index.shared' export type {PopupProps} from './index.shared' -// The sheet lives in a FullWindowOverlay, so it needs the window's insets. The -// nearest SafeAreaProvider can't supply them: a provider nested inside a -// react-native-screens scene (every modal route) re-measures to ~0. -const useWindowInsets = () => { - const local = useSafeAreaInsets() - const window = initialWindowMetrics?.insets - return { - bottom: Math.max(window?.bottom ?? 0, local.bottom), - top: Math.max(window?.top ?? 0, local.top), +// The one place the platform rule lives: on mobile every popup presents as a +// bottom sheet. Callers that need a mode regardless of platform - an overlay +// pinned to an input, a desktop-only cover - render that mode directly instead. +function Popup(props: PopupProps) { + if (isMobile) { + return ( + + {props.children} + + ) } -} - -function Backdrop(props: BottomSheetBackdropProps) { - return -} -const FullWindow = ({children}: {children?: React.ReactNode}): React.ReactNode => { - return isIOS ? {children} : children -} - -function DesktopPopupPositioned(props: PopupProps) { - const desktopStyles = useDesktopStyles() - return ( - - {props.onHidden ? ( - - {props.children} - - ) : ( - props.children - )} - - ) -} - -function PopupPositioned(props: PopupProps) { - // on mobile FloatingBox is the same portal + keyboard-dismiss overlay this needs - return isMobile ? : -} - -function PopupCentered(props: PopupProps) { - const desktopStyles = useDesktopStyles() - const {children, onHidden, style} = props - - const [mouseDownOnCover, setMouseDownOnCover] = React.useState(false) - return ( - {})}> - { - if (mouseDownOnCover) { - onHidden?.() - } - }} - onMouseDown={() => { - setMouseDownOnCover(true) - }} + if (props.intent === 'menu' && props.attachTo) { + return ( + - { - setMouseDownOnCover(false) - e.stopPropagation() - }} - onMouseUp={(e: React.BaseSyntheticEvent) => e.stopPropagation()} - > -
- {children} -
- - - - ) -} - -function stopBubbling(ev: React.MouseEvent) { - ev.stopPropagation() -} - -function PopupSheet(props: PopupProps) { - const nativeStyles = useNativeStyles() - const {children, footer, onHidden, snapPoints, style} = props - const {bottom: safeBottom, top: safeTop} = useWindowInsets() - const bottomRef = React.useRef(null) - // the sheet's content clears the home indicator plus a margin, so the last row - // never sits flush with the screen edge - const contentBottom = safeBottom + Styles.globalMargins.medium - const indicatorInsets = React.useMemo(() => ({bottom: contentBottom}), [contentBottom]) - - // the footer floats over the scrolled content down to the screen edge, so the - // caller's node must bring its own background and bottom safe-area padding - const renderFooter = React.useCallback( - (fp: BottomSheetFooterProps) => {footer}, - [footer] - ) - - React.useEffect(() => { - // the sheet covers the bottom of the screen, so a raised keyboard would hide it - Keyboard.dismiss() - bottomRef.current?.present() - return () => { - bottomRef.current?.forceClose() - bottomRef.current = null - } - }, []) - - const setBottomSheetRef = (sheet: BottomSheetModal | null) => { - bottomRef.current = sheet + {props.children} + + ) } + // a menu with nothing to anchor to falls back to the cover: the positioner + // can't measure a target and would render an invisible box return ( - dynamic sizing only: sheet hugs content and can't be dragged taller - snapPoints={snapPoints} - backgroundStyle={nativeStyles.modalBackground} - containerComponent={FullWindow} - handleStyle={nativeStyles.handleStyle} - handleIndicatorStyle={nativeStyles.handleIndicatorStyle} - style={nativeStyles.modalStyle} - backdropComponent={Backdrop} - onDismiss={onHidden} - // dynamic sizing clamps to the container (full window via FullWindowOverlay), - // so without this tall sheets cover the status bar - topInset={safeTop} - footerComponent={footer ? renderFooter : undefined} - > - {/* a scrollable must be the sheet's direct child: nesting one inside - BottomSheetView measures unbounded, so tall content clips instead of scrolling */} - - {children} - - + + {props.children} + ) } -function Popup(props: PopupProps) { - // sheets present on mount, so an explicitly hidden popup must not render - if (Object.hasOwn(props, 'visible') && !props.visible) { - return null - } - if (props.attachTo && (!isMobile || props.mobileAnchored)) { - return - } - if (isMobile) { - if (!props.onHidden) { - return {props.children} - } - return - } - return -} - -const useDesktopStyles = Styles.createStyleHook(theme => ({ - centeredContainer: { - maxHeight: '100%', - maxWidth: '100%', - }, - clipContainer: Styles.platformStyles({ - isElectron: { - ...Styles.desktopStyles.boxShadow, - ...Styles.globalStyles.flexBoxColumn, - backgroundColor: theme.white, - borderRadius: Styles.borderRadius, - flex: 1, - maxWidth: '100%', - position: 'relative', - }, - }), - cover: { - ...Styles.globalStyles.fillAbsolute, - alignSelf: 'stretch', - ...Styles.padding(Styles.globalMargins.large, Styles.globalMargins.large, Styles.globalMargins.small), - }, - positioned: Styles.platformStyles({ - isElectron: { - ...Styles.desktopStyles.boxShadow, - ...Styles.globalStyles.rounded, - overflowX: 'hidden', - overflowY: 'auto', - }, - }), -})) - -const useNativeStyles = Styles.createStyleHook( - theme => - ({ - handleIndicatorStyle: {backgroundColor: theme.black_40}, - handleStyle: {backgroundColor: theme.black_05_on_white}, - modalBackground: {backgroundColor: theme.black_05_on_white}, - modalStyle: Styles.platformStyles({ - isAndroid: { - elevation: 17, - shadowColor: theme.black_50OrBlack_40, - shadowOffset: {height: 5, width: 0}, - shadowOpacity: 1, - shadowRadius: 10, - }, - }), - }) as const -) - export default Popup diff --git a/shared/common-adapters/popup/modal-cover.desktop.tsx b/shared/common-adapters/popup/modal-cover.desktop.tsx new file mode 100644 index 000000000000..fd0bba624abc --- /dev/null +++ b/shared/common-adapters/popup/modal-cover.desktop.tsx @@ -0,0 +1,78 @@ +import * as React from 'react' +import * as Styles from '@/styles' +import {Box2} from '../box' +import {EscapeHandler} from '../key-event-handler.desktop' +import type {ModalCoverProps} from './index.shared' +export type {ModalCoverProps} from './index.shared' + +const noop = () => {} + +function stopBubbling(ev: React.MouseEvent) { + ev.stopPropagation() +} + +export function ModalCover(props: ModalCoverProps) { + const styles = useStyles() + const {children, onHidden, style} = props + + // a press that starts on the content and ends on the cover must not dismiss, + // so the cover only closes when its own mousedown was the one that opened + const [mouseDownOnCover, setMouseDownOnCover] = React.useState(false) + return ( + + { + if (mouseDownOnCover) { + onHidden?.() + } + }} + onMouseDown={() => { + setMouseDownOnCover(true) + }} + > + { + setMouseDownOnCover(false) + e.stopPropagation() + }} + onMouseUp={(e: React.BaseSyntheticEvent) => e.stopPropagation()} + > +
+ {children} +
+ + + + ) +} + +const useStyles = Styles.createStyleHook(theme => ({ + centeredContainer: { + maxHeight: '100%', + maxWidth: '100%', + }, + clipContainer: Styles.platformStyles({ + isElectron: { + ...Styles.desktopStyles.boxShadow, + ...Styles.globalStyles.flexBoxColumn, + backgroundColor: theme.white, + borderRadius: Styles.borderRadius, + flex: 1, + maxWidth: '100%', + position: 'relative', + }, + }), + cover: { + ...Styles.globalStyles.fillAbsolute, + alignSelf: 'stretch', + ...Styles.padding(Styles.globalMargins.large, Styles.globalMargins.large, Styles.globalMargins.small), + }, +})) + +export default ModalCover diff --git a/shared/common-adapters/popup/modal-cover.test.tsx b/shared/common-adapters/popup/modal-cover.test.tsx new file mode 100644 index 000000000000..406293353d03 --- /dev/null +++ b/shared/common-adapters/popup/modal-cover.test.tsx @@ -0,0 +1,92 @@ +/** @jest-environment jsdom */ +/// + +import {cleanup, fireEvent, render, screen} from '@testing-library/react' +import {Box2} from '../box' +import {GlobalKeyEventHandler} from '../key-event-handler.desktop' +import {ModalCover} from './modal-cover.desktop' + +const renderCover = (onHidden?: () => void) => { + const {container} = render( + + + content + + + ) + return { + content: screen.getByText('content'), + cover: container.firstElementChild as HTMLElement, + } +} + +const pressEscape = () => { + fireEvent.keyDown(document.body, {key: 'Escape'}) +} + +describe('ModalCover', () => { + afterEach(() => { + cleanup() + }) + + test('renders its children on the cover', () => { + const {content, cover} = renderCover(() => {}) + expect(cover.contains(content)).toBe(true) + }) + + test('a press that starts and ends on the cover dismisses', () => { + const onHidden = jest.fn() + const {cover} = renderCover(onHidden) + fireEvent.mouseDown(cover) + fireEvent.mouseUp(cover) + expect(onHidden).toHaveBeenCalledTimes(1) + }) + + test('a release on the cover with no press on it does not dismiss', () => { + const onHidden = jest.fn() + const {cover} = renderCover(onHidden) + fireEvent.mouseUp(cover) + expect(onHidden).not.toHaveBeenCalled() + }) + + // dragging a selection out of the content and releasing on the cover must not + // close the popup + test('a press that starts on the content and ends on the cover does not dismiss', () => { + const onHidden = jest.fn() + const {content, cover} = renderCover(onHidden) + fireEvent.mouseDown(content) + fireEvent.mouseUp(cover) + expect(onHidden).not.toHaveBeenCalled() + }) + + test('a release on the content does not dismiss even after pressing the cover', () => { + const onHidden = jest.fn() + const {content, cover} = renderCover(onHidden) + fireEvent.mouseDown(cover) + fireEvent.mouseUp(content) + expect(onHidden).not.toHaveBeenCalled() + }) + + test('escape dismisses', () => { + const onHidden = jest.fn() + renderCover(onHidden) + pressEscape() + expect(onHidden).toHaveBeenCalledTimes(1) + }) + + test('other keys do not dismiss', () => { + const onHidden = jest.fn() + renderCover(onHidden) + fireEvent.keyDown(document.body, {key: 'Enter'}) + expect(onHidden).not.toHaveBeenCalled() + }) + + test('without onHidden neither escape nor a cover press throws', () => { + const {cover} = renderCover() + expect(() => { + fireEvent.mouseDown(cover) + fireEvent.mouseUp(cover) + pressEscape() + }).not.toThrow() + }) +}) diff --git a/shared/common-adapters/popup/modal-cover.tsx b/shared/common-adapters/popup/modal-cover.tsx new file mode 100644 index 000000000000..8d40192a28f9 --- /dev/null +++ b/shared/common-adapters/popup/modal-cover.tsx @@ -0,0 +1,7 @@ +import type {ModalCoverProps} from './index.shared' +export type {ModalCoverProps} from './index.shared' + +// The cover is a desktop presentation; mobile callers use Sheet. +export const ModalCover = (_p: ModalCoverProps) => null + +export default ModalCover diff --git a/shared/common-adapters/popup/sheet.native.tsx b/shared/common-adapters/popup/sheet.native.tsx new file mode 100644 index 000000000000..c8c43ae46199 --- /dev/null +++ b/shared/common-adapters/popup/sheet.native.tsx @@ -0,0 +1,127 @@ +import * as React from 'react' +import * as Styles from '@/styles' +import { + BottomSheetModal, + BottomSheetScrollView, + BottomSheetBackdrop, + BottomSheetFooter, + type BottomSheetBackdropProps, + type BottomSheetFooterProps, +} from '@gorhom/bottom-sheet' +import {useSafeAreaInsets} from '../safe-area-view' +import {initialWindowMetrics} from 'react-native-safe-area-context' +import {FullWindowOverlay} from 'react-native-screens' +import {Keyboard} from 'react-native' +import type {SheetProps} from './index.shared' +export type {SheetProps} from './index.shared' + +// The sheet lives in a FullWindowOverlay, so it needs the window's insets. The +// nearest SafeAreaProvider can't supply them: a provider nested inside a +// react-native-screens scene (every modal route) re-measures to ~0. +const useWindowInsets = () => { + const local = useSafeAreaInsets() + const window = initialWindowMetrics?.insets + return { + bottom: Math.max(window?.bottom ?? 0, local.bottom), + top: Math.max(window?.top ?? 0, local.top), + } +} + +function Backdrop(props: BottomSheetBackdropProps) { + return +} + +const FullWindow = ({children}: {children?: React.ReactNode}): React.ReactNode => { + return isIOS ? {children} : children +} + +export function Sheet(props: SheetProps) { + const styles = useStyles() + const {children, footer, onHidden, snapPoints, style} = props + const {bottom: safeBottom, top: safeTop} = useWindowInsets() + const bottomRef = React.useRef | null>(null) + // the sheet's content clears the home indicator plus a margin, so the last row + // never sits flush with the screen edge + const contentBottom = safeBottom + Styles.globalMargins.medium + const indicatorInsets = React.useMemo(() => ({bottom: contentBottom}), [contentBottom]) + + // the footer floats over the scrolled content down to the screen edge, so the + // caller's node must bring its own background and bottom safe-area padding + const renderFooter = React.useCallback( + (fp: BottomSheetFooterProps) => {footer}, + [footer] + ) + + React.useEffect(() => { + // the sheet covers the bottom of the screen, so a raised keyboard would hide it + Keyboard.dismiss() + bottomRef.current?.present() + return () => { + bottomRef.current?.forceClose() + bottomRef.current = null + } + }, []) + + const setBottomSheetRef = (sheet: React.ComponentRef | null) => { + bottomRef.current = sheet + } + + return ( + dynamic sizing only: sheet hugs content and can't be dragged taller + snapPoints={snapPoints} + backgroundStyle={styles.modalBackground} + containerComponent={FullWindow} + handleStyle={styles.handleStyle} + handleIndicatorStyle={styles.handleIndicatorStyle} + style={styles.modalStyle} + backdropComponent={Backdrop} + onDismiss={onHidden} + // dynamic sizing clamps to the container (full window via FullWindowOverlay), + // so without this tall sheets cover the status bar + topInset={safeTop} + footerComponent={footer ? renderFooter : undefined} + > + {/* a scrollable must be the sheet's direct child: nesting one inside + BottomSheetView measures unbounded, so tall content clips instead of scrolling */} + + {children} + + + ) +} + +const useStyles = Styles.createStyleHook( + theme => + ({ + handleIndicatorStyle: {backgroundColor: theme.black_40}, + handleStyle: {backgroundColor: theme.black_05_on_white}, + modalBackground: {backgroundColor: theme.black_05_on_white}, + modalStyle: Styles.platformStyles({ + isAndroid: { + elevation: 17, + shadowColor: theme.black_50OrBlack_40, + shadowOffset: {height: 5, width: 0}, + shadowOpacity: 1, + shadowRadius: 10, + }, + }), + }) as const +) + +export default Sheet diff --git a/shared/common-adapters/popup/sheet.tsx b/shared/common-adapters/popup/sheet.tsx new file mode 100644 index 000000000000..6a4f5ea4b954 --- /dev/null +++ b/shared/common-adapters/popup/sheet.tsx @@ -0,0 +1,7 @@ +import type {SheetProps} from './index.shared' +export type {SheetProps} from './index.shared' + +// Sheets are a mobile presentation; desktop callers use ModalCover or AnchoredPopup. +export const Sheet = (_p: SheetProps) => null + +export default Sheet diff --git a/shared/common-adapters/popup/use-popup.test.tsx b/shared/common-adapters/popup/use-popup.test.tsx index ad2b1ead69cf..94a0db2fbbe5 100644 --- a/shared/common-adapters/popup/use-popup.test.tsx +++ b/shared/common-adapters/popup/use-popup.test.tsx @@ -91,7 +91,9 @@ describe('usePopup2', () => { expect(result.current.popupAnchor).toBe(popupAnchor) }) - test('desktop passes the anchor ref to makePopup, mobile does not', () => { + // the platform rule lives in the popup modules, not here: the anchor is handed + // over on both platforms and the mobile presentations simply never measure it + test('passes the anchor ref to makePopup on every platform', () => { const parms: Array = [] const spyMake = (p: Popup2Parms) => { parms.push(p) @@ -110,7 +112,7 @@ describe('usePopup2', () => { act(() => { mobile.result.current.showPopup() }) - expect(parms[0]?.attachTo).toBeUndefined() + expect(parms[0]?.attachTo).toBe(mobile.result.current.popupAnchor) }) test('a new makePopup identity re-renders the popup contents while showing', () => { diff --git a/shared/common-adapters/popup/use-popup.tsx b/shared/common-adapters/popup/use-popup.tsx index 5d8f64b4fde1..14c6627b6e2e 100644 --- a/shared/common-adapters/popup/use-popup.tsx +++ b/shared/common-adapters/popup/use-popup.tsx @@ -14,7 +14,6 @@ export const usePopup2 = (makePopup: (p: Popup2Parms) => React.ReactElement | nu const wasMakePopupRef = React.useRef<(p: Popup2Parms) => React.ReactElement | null>(makePopup) const [popup, setPopup] = React.useState(null) const popupAnchor = React.useRef(null) - const attachTo = isMobile ? undefined : popupAnchor const lastToggle = React.useRef(0) const [hidePopup] = React.useState(() => () => { @@ -38,9 +37,9 @@ export const usePopup2 = (makePopup: (p: Popup2Parms) => React.ReactElement | nu React.useEffect(() => { if (makePopup !== wasMakePopupRef.current || showingPopup !== !!popup) { wasMakePopupRef.current = makePopup - setPopup(showingPopup ? makePopup({attachTo, hidePopup, showPopup}) : null) + setPopup(showingPopup ? makePopup({attachTo: popupAnchor, hidePopup, showPopup}) : null) } - }, [attachTo, hidePopup, makePopup, popup, setPopup, showPopup, showingPopup]) + }, [hidePopup, makePopup, popup, setPopup, showPopup, showingPopup]) return {hidePopup, popup, popupAnchor, showPopup, showingPopup, togglePopup} } diff --git a/shared/common-adapters/toast.tsx b/shared/common-adapters/toast.tsx index 1f3aa6f565d3..4b59d08e0a9c 100644 --- a/shared/common-adapters/toast.tsx +++ b/shared/common-adapters/toast.tsx @@ -2,7 +2,8 @@ import * as C from '@/constants' import * as React from 'react' import * as Styles from '@/styles' import {Box2} from './box' -import Popup from './popup' +import {AnchoredPopup} from './popup/anchored' +import {Portal} from './portal' import {Animated as NativeAnimated, Easing as NativeEasing, useColorScheme} from 'react-native' import {colors, darkColors} from '@/styles/colors' import './toast.css' @@ -19,8 +20,9 @@ type Props = { } const Kb = { + AnchoredPopup, Box2, - Popup, + Portal, } const positionFallbacks = [] as const @@ -120,7 +122,7 @@ const Toast = (props: Props) => { if (!isMobile) { return ( - { > {props.children} - + ) } return shouldRender ? ( - + { {props.children} - + ) : null } diff --git a/shared/fs/common/sfmi-popup.tsx b/shared/fs/common/sfmi-popup.tsx index 2663d6b37e9d..c6e565054641 100644 --- a/shared/fs/common/sfmi-popup.tsx +++ b/shared/fs/common/sfmi-popup.tsx @@ -26,7 +26,13 @@ const SFMIPopup = (props: Props) => { const {attachTo, hidePopup} = p return ( - + { if (!isMobile) { return ( - {}} style={styles.desktopCover}> + // no onHidden: the reset countdown can't be dismissed by escape or a click away + {content} - + ) } diff --git a/shared/provision/code-page/container.tsx b/shared/provision/code-page/container.tsx index 20ccf2483acc..f0ee096ca8d5 100644 --- a/shared/provision/code-page/container.tsx +++ b/shared/provision/code-page/container.tsx @@ -175,7 +175,7 @@ const CodePageContainer = (op: OwnProps) => { {!inModal && otherDevice.type === 'desktop' && heyWaitBanner()} {!inModal && troubleshooting && ( - setTroubleshooting(false)} propagateOutsideClicks={true}> + setTroubleshooting(false)}> {troubleshootingContent()} )} diff --git a/shared/teams/common/enable-contacts.tsx b/shared/teams/common/enable-contacts.tsx index 1dd524f07e8a..3eec789069e2 100644 --- a/shared/teams/common/enable-contacts.tsx +++ b/shared/teams/common/enable-contacts.tsx @@ -28,7 +28,7 @@ const EnableContactsPopup = ({noAccess, onClose}: {noAccess: boolean; onClose: ( } return showingPopup ? ( - + diff --git a/shared/teams/common/selection-popup.tsx b/shared/teams/common/selection-popup.tsx index 4a98af7c65fb..c513f41ffa85 100644 --- a/shared/teams/common/selection-popup.tsx +++ b/shared/teams/common/selection-popup.tsx @@ -114,7 +114,7 @@ const JointSelectionPopup = (props: JointSelectionPopupProps) => { return isMobile ? ( <> { 48 ? height - 48 - bottom : -bottom}} />} - {popup} + {popup} ) : ( popup diff --git a/shared/teams/common/use-autocompleter.tsx b/shared/teams/common/use-autocompleter.tsx index d4eb68d1cab7..b7898b816f49 100644 --- a/shared/teams/common/use-autocompleter.tsx +++ b/shared/teams/common/use-autocompleter.tsx @@ -29,6 +29,7 @@ function useAutocompleter( const {attachTo, hidePopup} = p return ( { ) return ( - + {content} ) diff --git a/shared/teams/emojis/add-alias.tsx b/shared/teams/emojis/add-alias.tsx index 712e05ea1048..c73f18768bd9 100644 --- a/shared/teams/emojis/add-alias.tsx +++ b/shared/teams/emojis/add-alias.tsx @@ -207,6 +207,7 @@ const ChooseEmojiDesktop = (props: ChooseEmojiProps) => { const {attachTo, hidePopup} = p return ( {open && ( - {})} hideKeyboard={true} @@ -428,7 +427,7 @@ export function FloatingRolePicker - + )} ) diff --git a/shared/teams/team/settings-tab/channel-popup.tsx b/shared/teams/team/settings-tab/channel-popup.tsx index 61530f1c72ed..64dafe08f0a0 100644 --- a/shared/teams/team/settings-tab/channel-popup.tsx +++ b/shared/teams/team/settings-tab/channel-popup.tsx @@ -45,7 +45,7 @@ const ChannelPopup = (props: Props) => { const onAdd = () => onComplete(selected) return ( - + diff --git a/shared/tsconfig.native.json b/shared/tsconfig.native.json index 399783d67966..5e0ec8e33892 100644 --- a/shared/tsconfig.native.json +++ b/shared/tsconfig.native.json @@ -22,10 +22,12 @@ "./chat/conversation/messages/text/coinflip/results.test.tsx", "./common-adapters/markdown/index.test.tsx", "./common-adapters/banner.test.tsx", + "./common-adapters/floating-menu/index.test.tsx", "./common-adapters/hot-key.test.tsx", "./common-adapters/icon.test.tsx", "./common-adapters/name-with-icon.test.tsx", "./common-adapters/popup/floating-box/relative-floating-box.test.tsx", + "./common-adapters/popup/modal-cover.test.tsx", "./common-adapters/save-indicator.test.tsx", "./common-adapters/text.test.tsx", "./devices/index.test.tsx",