From 6026d985989172ffd4bfc83e48a4332df4dac44d Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:39:53 -0700 Subject: [PATCH 1/3] Half the editor was on the phone and none of it could be touched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as "quite a few options don't function on mobile", and measured in a real touch emulation rather than a narrow window — the distinction matters, because two of the three causes need `pointer: coarse` to appear at all. Three causes, one theme: the chrome was built for a pointer that hovers. The toolbar reveals itself on :hover — flex-wrap: wrap, overflow: visible. A touchscreen never hovers, so that rule never fired and the row stayed clipped at overflow: hidden permanently. Fourteen of its twenty-three controls could not be reached by any gesture: every heading, both lists, all three alignments, link, image, table, outline, and both AI passes. The faded right edge promised there was more that way, and there was no way. It now scrolls sideways on a coarse pointer, keeping every control, exactly as .petal-chrome-strip does one line above it — the same trade that comment already argued for, applied to the row it was comparing itself to. That move was only safe once the panels could get out. Every dropdown in the editor chrome — tone, export, colour, highlight, size — was already broken on a phone for a subtler reason: `position: fixed` is relative to the viewport only while no ancestor establishes a containing block, and mask-image does, exactly like transform. Both scrollers fade their edges with a mask. So on any screen narrow enough for the fade to appear, the menu was pulled back inside the very box it was escaping, painted under the toolbar and untappable. The comment in anchoredMenu.ts said "nothing clips a fixed box unless an ancestor has a transform, and none of the editor's chrome does"; it was true when it was written and had quietly stopped being true. The panels now portal to , where nothing above them can clip, stack over or contain them whatever the chrome does with masks later, and the outside-tap tests ask about both halves. The kitten took the last two. She covered "Español" and "I am learning Português" in the drawer, and "Hide falling petals" in the status bar outright — three sample points across it, all three landing on the cat. She already knows how to yield: useCardOverlap fades her for cards and for anything with the modal role, and the drawer is the one overlay that has neither, being navigation rather than something opened on purpose. It is named there now. The status bar is a different problem — it is always present, so yielding to it would mean fading forever — and the honest fix is to sit above it on a narrow screen instead of negotiating with it every frame. Measuring, not eyeballing: a sweep of every visible, enabled control reports zero off-screen and zero blocked, against fourteen and three before. Desktop is deliberately untouched — at rest the slim clipped line with its mask, on hover the wrapped row with all twenty-three buttons on screen — and the edge measurement ChromeStrip already did is now shared with the toolbar rather than written twice. Claude-Session: https://claude.ai/code/session_01GJHNvirh7Hzhc9RL3HAvz7 --- .../components/Companion/useCardOverlap.ts | 9 ++- web/src/components/Editor/ChromeStrip.tsx | 37 ++-------- web/src/components/Editor/ToneSelect.tsx | 18 +++-- web/src/components/Editor/anchoredMenu.ts | 37 ++++++++-- web/src/components/Editor/useScrollEdge.ts | 51 +++++++++++++ web/src/components/Export/ExportMenu.tsx | 18 +++-- web/src/components/Toolbar/Toolbar.tsx | 73 +++++++++++++------ web/src/index.css | 69 ++++++++++++++++++ 8 files changed, 241 insertions(+), 71 deletions(-) create mode 100644 web/src/components/Editor/useScrollEdge.ts 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 (