diff --git a/web/src/components/Companion/useCardOverlap.ts b/web/src/components/Companion/useCardOverlap.ts index 136e4f0..e77ce68 100644 --- a/web/src/components/Companion/useCardOverlap.ts +++ b/web/src/components/Companion/useCardOverlap.ts @@ -17,7 +17,14 @@ const POLL_MS = 500 const HOLD_PX = 24 const CARD = '.petal-rail-card' -const MODAL = '[role="dialog"][aria-modal="true"]' +// The mobile sidebar drawer is named outright because it is the one overlay that +// isn't a dialog. It slides over the page behind a scrim exactly as History and +// Garden do, but it is the app's own navigation rather than something opened on +// purpose, so it carries no modal role for the selector above to catch — and the +// kitten sat in its bottom corner, over the last two rows of the language +// picker. On a 390px phone that put "Español" and "I am learning Português" +// under the halo: visibly there, and only partly tappable. +const MODAL = '[role="dialog"][aria-modal="true"], .petal-sidebar.petal-drawer-open' export interface CardOverlap { // A suggestion card reaches the mascot. It should get out of the way, but may diff --git a/web/src/components/Editor/ChromeStrip.tsx b/web/src/components/Editor/ChromeStrip.tsx index b1258dd..17d931f 100644 --- a/web/src/components/Editor/ChromeStrip.tsx +++ b/web/src/components/Editor/ChromeStrip.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from 'react' +import { useScrollEdge } from './useScrollEdge' // ChromeStrip is the row of document pills (tone, history, export) on a screen // too narrow to hold them. It scrolls within itself rather than letting the @@ -15,7 +15,9 @@ import { useCallback, useEffect, useRef, useState } from 'react' // The fade is a mask rather than a gradient overlay so it works on whatever is // behind it (the cream page, the night theme, a falling petal) without knowing // the background colour. -type Edge = 'none' | 'left' | 'right' | 'both' +// +// The measuring itself lives in useScrollEdge, shared with the formatting +// toolbar — which has to say the same thing for the same reason. interface Props { className?: string @@ -23,36 +25,7 @@ interface Props { } export function ChromeStrip({ className = '', children }: Props) { - const ref = useRef(null) - const [edge, setEdge] = useState('none') - - // A pixel of slack: scrollLeft is fractional under browser zoom and on - // high-DPI screens, so an exactly-scrolled-to-the-end strip can report - // something like 0.5px remaining and fade an edge that has nothing behind it. - const measure = useCallback(() => { - const el = ref.current - if (!el) return - const more = el.scrollWidth - el.clientWidth - el.scrollLeft > 1 - const less = el.scrollLeft > 1 - setEdge(less && more ? 'both' : less ? 'left' : more ? 'right' : 'none') - }, []) - - useEffect(() => { - const el = ref.current - if (!el) return - measure() - el.addEventListener('scroll', measure, { passive: true }) - // Both halves of "does it fit" can change without a scroll: the window - // resizes, or the labels themselves change when she switches her pair - // language and every pill in the row grows or shrinks at once. - const ro = new ResizeObserver(measure) - ro.observe(el) - for (const child of Array.from(el.children)) ro.observe(child) - return () => { - el.removeEventListener('scroll', measure) - ro.disconnect() - } - }, [measure]) + const { ref, edge } = useScrollEdge() return (
diff --git a/web/src/components/Editor/ToneSelect.tsx b/web/src/components/Editor/ToneSelect.tsx index b7c7645..12a1790 100644 --- a/web/src/components/Editor/ToneSelect.tsx +++ b/web/src/components/Editor/ToneSelect.tsx @@ -1,4 +1,5 @@ import { useEffect, useRef, useState } from 'react' +import { createPortal } from 'react-dom' import { usePack } from '../../i18n' import { useAnchoredMenu } from './anchoredMenu' @@ -36,18 +37,20 @@ export function ToneSelect({ value, onChange }: Props) { const pk = usePack() const [open, setOpen] = useState(false) const ref = useRef(null) - const { triggerRef, style: menuStyle } = useAnchoredMenu(open, 200) + const { triggerRef, panelRef, style: menuStyle } = useAnchoredMenu(open, 200) const current = TONES.find((t) => t.value === value) ?? TONES[0] - // Click outside closes the menu. + // Click outside closes the menu. The list is portalled to , so a tap on + // an option is not inside `ref` and has to be asked about separately. useEffect(() => { if (!open) return const onDown = (e: MouseEvent) => { - if (!ref.current?.contains(e.target as Node)) setOpen(false) + const target = e.target as Node + if (!ref.current?.contains(target) && !panelRef.current?.contains(target)) setOpen(false) } document.addEventListener('mousedown', onDown) return () => document.removeEventListener('mousedown', onDown) - }, [open]) + }, [open, panelRef]) return (
@@ -75,8 +78,10 @@ export function ToneSelect({ value, onChange }: Props) { - {open && ( + {open && + createPortal(
) })} -
+
, + document.body, )}
) diff --git a/web/src/components/Editor/anchoredMenu.ts b/web/src/components/Editor/anchoredMenu.ts index 6fc5f70..99a33e7 100644 --- a/web/src/components/Editor/anchoredMenu.ts +++ b/web/src/components/Editor/anchoredMenu.ts @@ -6,15 +6,40 @@ import { useLayoutEffect, useRef, useState, type CSSProperties } from 'react' // button's wrapper, which was fine until that wrapper became ChromeStrip — a // horizontal scroller, and so a box that clips what overflows it. An absolute // menu inside it is 36px tall and scrolls away with the pills. Positioning the -// menu against the viewport instead takes it out of the strip's hands entirely: -// nothing clips a fixed box unless an ancestor has a transform, and none of the -// editor's chrome does. +// menu against the viewport instead takes it out of the strip's hands entirely. +// +// Or rather: it does once the menu is also *portalled out* of it, which is the +// part this originally got wrong. `position: fixed` is only relative to the +// viewport while no ancestor establishes a containing block for it — and a +// `mask-image` does, exactly like a transform. Both scrollers fade their edges +// with a mask (that is how each says "there is more this way"), so on any screen +// narrow enough for the fade to appear — i.e. every phone — the menu was pulled +// back inside the very box it was trying to escape: painted underneath the +// toolbar, and untappable. It looked open and did nothing. +// +// So the panel is rendered through a portal into . Nothing above it can +// clip it, stack over it, or contain it, whatever the chrome does with masks +// later. `panelRef` is returned for the outside-tap test, which can no longer +// rely on the panel being a DOM descendant of the trigger's wrapper. // // The trade is that a fixed box doesn't follow its anchor, so anything that // moves the button — the page scrolling under it, the strip scrolling, the // window resizing — has to re-place the menu. -export function useAnchoredMenu(open: boolean, width: number) { - const triggerRef = useRef(null) +// +// The element type is a parameter because the two kinds of caller anchor +// against different things: the tone and export pills hand it their own +// - {open && ( + {open && + createPortal(
-
+ , + document.body, )} ) diff --git a/web/src/components/Toolbar/Toolbar.tsx b/web/src/components/Toolbar/Toolbar.tsx index f0b4dc2..e8a4e5c 100644 --- a/web/src/components/Toolbar/Toolbar.tsx +++ b/web/src/components/Toolbar/Toolbar.tsx @@ -1,7 +1,10 @@ import type { Editor } from '@tiptap/react' import { useEditorState } from '@tiptap/react' import { useEffect, useRef, useState } from 'react' +import { createPortal } from 'react-dom' import { uploadImageInto } from '../Editor/EditorCore' +import { useAnchoredMenu } from '../Editor/anchoredMenu' +import { useScrollEdge } from '../Editor/useScrollEdge' import { usePack } from '../../i18n' interface Props { @@ -56,9 +59,26 @@ const Divider = () => ( ) -// A popover anchored under its trigger. The trigger + panel share a relative -// wrapper; `open`/`onClose` are owned by the toolbar so only one is open at once. -// A pointer-down outside the wrapper closes it. +// A popover anchored under its trigger. The trigger + panel share a wrapper; +// `open`/`onClose` are owned by the toolbar so only one is open at once. A +// pointer-down outside the wrapper closes it. +// +// The panel is placed in viewport coordinates rather than absolutely inside +// that wrapper, for the same two reasons ChromeStrip's menus were (see +// useAnchoredMenu) — and on a phone both of them bite at once: +// +// * The toolbar clips what overflows it. On a desktop that clip is lifted on +// hover, which is where an absolutely-positioned panel got away with it for +// as long as it did; a touchscreen never hovers, so tapping A or H opened a +// panel that was simply not on the screen. The button lit up and nothing +// else happened, which is the worst shape a bug can take — it reads as the +// feature not existing. +// * `left-0` hangs a 200px panel off the right edge of a 390px phone when its +// trigger sits near the end of the row. useAnchoredMenu clamps it back +// inside the window instead. +// +// The panel stays a DOM child of the wrapper (fixed, not portalled) so the +// outside-tap test below keeps working on containment alone. function Popover({ open, onClose, @@ -72,32 +92,37 @@ function Popover({ children: React.ReactNode width?: number }) { - const ref = useRef(null) + const { triggerRef: ref, panelRef, style } = useAnchoredMenu(open, width) useEffect(() => { if (!open) return const onDown = (e: MouseEvent) => { - if (!ref.current?.contains(e.target as Node)) onClose() + const target = e.target as Node + // The panel is portalled to , so "inside" is either half. + if (!ref.current?.contains(target) && !panelRef.current?.contains(target)) onClose() } document.addEventListener('mousedown', onDown) return () => document.removeEventListener('mousedown', onDown) - }, [open, onClose]) + }, [open, onClose, ref, panelRef]) return ( -
+
{trigger} - {open && ( -
- {children} -
- )} + {open && + createPortal( +
+ {children} +
, + document.body, + )}
) } @@ -175,6 +200,10 @@ export function Toolbar({ editor, onVoiceCheck, voicing, onCollocationCheck, col const [menu, setMenu] = useState<'color' | 'highlight' | 'size' | 'link' | 'table' | 'outline' | null>(null) const [linkUrl, setLinkUrl] = useState('') const fileInputRef = useRef(null) + // Which end of the row still has controls behind it. Only ever visible on a + // coarse pointer, where the row scrolls instead of expanding on hover — see + // the .petal-toolbar rules in index.css. + const { ref: toolbarRef, edge } = useScrollEdge() const state = useEditorState({ editor, @@ -255,6 +284,8 @@ export function Toolbar({ editor, onVoiceCheck, voicing, onCollocationCheck, col return (