diff --git a/internal/auth/pairlang_test.go b/internal/auth/pairlang_test.go index 2e66229..39f3c8a 100644 --- a/internal/auth/pairlang_test.go +++ b/internal/auth/pairlang_test.go @@ -180,14 +180,17 @@ func TestDirectionRoundTrip(t *testing.T) { } } -// The refusal this axis exists to make: a pair with no word list cannot be -// learned toward, however good its langpack is. fr, es and pt-PT all have copy, -// voices and spelling dictionaries — and nothing that could segment a sentence -// or read from that language into English, which is what a learner needs. +// The refusal this axis exists to make: a pair with no learner-side data cannot +// be learned toward, however good its langpack is. fr and es have copy, voices +// and spelling dictionaries, and no `learner` block in their packs to offer the +// choice with — so the server keeps saying no until one is written. +// +// pt-PT is deliberately no longer in this list; see TestLearnerDirectionForPtPT +// below and the argument in `learnerPairs`. func TestLearnerDirectionRefusedForPairsWithoutData(t *testing.T) { _, users, _ := newStores(t) - for _, lang := range []string{"pt-PT", "fr", "es"} { + for _, lang := range []string{"fr", "es"} { if err := users.SetPair("bob", lang, DirectionLearningEn); err != nil { t.Fatalf("set %s: %v", lang, err) } @@ -201,6 +204,37 @@ func TestLearnerDirectionRefusedForPairsWithoutData(t *testing.T) { } } +// The other direction of that same rule, and the one a native English speaker +// writing Portuguese depends on. +// +// This is not only a settings toggle: `direction` is what decides which language +// Petal *explains* in (see suggestions.targetFor), so an account that cannot +// reach learning_pair gets its Portuguese annotated in Portuguese with no way to +// ask for English. Pinned in both directions — the move must take, and it must +// still be there when the account is read back. +func TestLearnerDirectionForPtPT(t *testing.T) { + _, users, _ := newStores(t) + + if err := users.SetPair("bob", "pt-PT", DirectionLearningEn); err != nil { + t.Fatalf("set pt-PT: %v", err) + } + if w := patchMe(t, users, "bob", `{"direction":"learning_pair"}`); w.Code != http.StatusOK { + t.Fatalf("status = %d (%s), want 200", w.Code, w.Body.String()) + } + u, _ := users.Get("bob") + if u.Direction != DirectionLearningPair || u.PairLang != "pt-PT" { + t.Fatalf("account = %+v, want pt-PT learning_pair", u) + } + + // And it can be turned back, the same as zh. + if w := patchMe(t, users, "bob", `{"direction":"learning_en"}`); w.Code != http.StatusOK { + t.Fatalf("turn back: status = %d (%s)", w.Code, w.Body.String()) + } + if u, _ := users.Get("bob"); u.Direction != DirectionLearningEn { + t.Fatalf("direction = %q after turning back", u.Direction) + } +} + // The two-field combination the handler validates as one decision. An account // already learning Chinese that asks only to change pair is asking for a state // neither field names on its own — French with segmentation — and it must not diff --git a/internal/auth/users.go b/internal/auth/users.go index afa4078..955d8ab 100644 --- a/internal/auth/users.go +++ b/internal/auth/users.go @@ -104,19 +104,34 @@ const ( // The pairs whose *learner* direction Petal can actually serve, which is a // narrower thing than a shipped pair and narrower again than a langpack. // -// Turning a pair around needs data no langpack carries: a word list to segment -// with, and a dictionary that reads from the pair language into English. Chinese -// has both as of Phase 26 (CC-CEDICT + jieba); French, Spanish and Portuguese -// have neither yet, and — unlike a missing pack, which leaves a writer looking -// at copy she cannot read — a missing word list would leave her looking at an -// editor that silently does nothing when she hovers. Both are bad; only one is -// legible as a bug. So the server refuses, for the same reason and by the same -// mechanism as `shippedPairs`. +// Turning a pair around needs data no langpack carries: a way to find word +// boundaries, and a dictionary that reads from the pair language into English. A +// pair missing either would leave a writer looking at an editor that silently +// does nothing when she hovers — worse than a missing pack, which at least reads +// as a bug rather than as an absence. So the server refuses, for the same reason +// and by the same mechanism as `shippedPairs`. // -// This list is expected to grow one pair at a time and never to be inferred: -// segmentation is a property of a writing system, and there is no rule that -// derives "has a word list" from a language code. -var learnerPairs = []string{"zh"} +// Chinese has both as of Phase 26 (CC-CEDICT + jieba). Portuguese turns out to +// have both as well, and the original note here — "French, Spanish and +// Portuguese have neither" — was written one phase too early to see it: +// +// - Word boundaries are spaces. The megabyte word list jieba needs is a +// property of a writing system that doesn't use them, not a debt every pair +// owes; a Latin-script pair needs nothing loaded to be segmented. +// - The dictionary arrived with dict.db, which reads pt→en as readily as +// en→pt (see lexicon.dreamProvider.reverse). The reverse lookup the hover +// and the word card need is already there and already answering. +// +// So the pair a native English speaker learning Portuguese needs is real, and +// what was actually blocking it was this list. French and Spanish clear the same +// two bars through the same dict.db; they are held back only by their packs +// carrying no `learner` copy yet (see Pack.learner), which is a translation +// question rather than a data one. +// +// This list is still expected to grow one pair at a time and never to be +// inferred: segmentation is a property of a writing system, and there is no rule +// that derives "has a word list" from a language code. +var learnerPairs = []string{"zh", "pt-PT"} // SupportsLearnerDirection reports whether a pair can be turned around. func SupportsLearnerDirection(lang string) bool { diff --git a/internal/lexicon/handlers.go b/internal/lexicon/handlers.go index 837ade1..5b7960b 100644 --- a/internal/lexicon/handlers.go +++ b/internal/lexicon/handlers.go @@ -49,10 +49,19 @@ func (h *Handler) GlossRoutes() chi.Router { // It does not go through [Handler.providerFor], and that is not an oversight. // providerFor picks a dictionary by the writer's *pair*, to answer "what does // this English word mean in her language" — a question whose answer differs per -// pair. This endpoint asks the opposite question of exactly one language, and -// [auth.SupportsLearnerDirection] already guarantees that language is Chinese. -// Routing it through the pair would add a database read per hover to choose -// between one option and itself. +// pair. This endpoint asks the opposite question of exactly one language: it +// reads hanzi, and hanzi are Chinese whoever is looking them up. Routing it +// through the pair would add a database read per hover to choose between one +// option and itself. +// +// What no longer holds is the reason this used to give — that +// [auth.SupportsLearnerDirection] guarantees the caller is on the zh pair. Since +// Portuguese joined `learnerPairs` a learning_pair account may be Portuguese, so +// the guarantee now comes from the *caller*: the client only ever asks this +// route about a token its Chinese segmenter found, and that segmenter is loaded +// only for the zh pair (see useSegmenter in App.tsx). A stray lookup is still +// answered safely — a word the Chinese dictionary has never heard of is a 200 +// with empty lists, exactly like any other miss. func (h *Handler) HanziRoutes() chi.Router { r := chi.NewRouter() r.Get("/{word}", h.hanzi) diff --git a/internal/suggestions/doclang_test.go b/internal/suggestions/doclang_test.go index 3c3349c..2dc752b 100644 --- a/internal/suggestions/doclang_test.go +++ b/internal/suggestions/doclang_test.go @@ -119,8 +119,36 @@ func TestDocumentLangNeedsCorroboration(t *testing.T) { } } -// The two language decisions are genuinely independent, and only the zh pair can -// prove it today — it is the one pair that can be travelled in both directions. +// The Portuguese half of the same rule, and the bug it was reported as: "the +// Portuguese option isn't translating the advice in English — it's just +// reprinting Portuguese." +// +// Nothing was wrong with targetFor when that was reported. It was reading a +// direction the account could not leave: `learnerPairs` held only zh, so every +// pt-PT writer was learning_en by force and this function correctly explained a +// Portuguese document in Portuguese. Pinned here rather than only in the auth +// package because this is where the consequence actually lands — the language +// the writer reads her advice in. +func TestTargetExplainsPortugueseInEnglishForALearner(t *testing.T) { + learner := targetFor("pt-PT", auth.DirectionLearningPair, docLangPair) + if learner.Correct.Code != "pt-PT" { + t.Fatalf("corrected in %s, want the document's own Portuguese", learner.Correct.Code) + } + if learner.Explain.Code != "en" { + t.Fatalf("explained in %s, want English", learner.Explain.Code) + } + + // And the native Portuguese speaker practising English is untouched: her + // Portuguese is still explained in Portuguese. + native := targetFor("pt-PT", auth.DirectionLearningEn, docLangPair) + if native.Correct.Code != "pt-PT" || native.Explain.Code != "pt-PT" { + t.Fatalf("learning_en on a Portuguese document: correct=%s explain=%s", native.Correct.Code, native.Explain.Code) + } +} + +// The two language decisions are genuinely independent, and zh was the first +// pair that could prove it — the first that could be travelled in both +// directions. // // A Mandarin native practising English who writes Chinese wants Chinese // corrections explained in Chinese. An English native learning Chinese who writes diff --git a/web/src/App.tsx b/web/src/App.tsx index 9326d38..15f961d 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -118,7 +118,13 @@ export default function App() { // Mandarin native drafting English quotes Chinese constantly, and none of that // is what segmentation is for. Declared above the checkpoint because the // offline 错别字 pass reads it. - const segmenter = useSegmenter(me?.direction === 'learning_pair') + // + // Both halves of the gate matter now that Chinese is not the only pair with a + // learner direction. `learning_pair` alone used to imply zh; a writer learning + // Portuguese is also learning_pair and has no use for a megabyte of Chinese + // word list — nor for the hanzi hover it turns on, which would ask /api/hanzi + // about Portuguese words. + const segmenter = useSegmenter(me?.direction === 'learning_pair' && me?.pair_lang === 'zh') const { suggestions, 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/EditorCore.tsx b/web/src/components/Editor/EditorCore.tsx index b596914..142103f 100644 --- a/web/src/components/Editor/EditorCore.tsx +++ b/web/src/components/Editor/EditorCore.tsx @@ -1582,6 +1582,7 @@ export function EditorCore({ saved={wordInfo.saved} onToggleSave={toggleSaveWord} pinyin={wordInfo.pinyin} + lang={docLocale(wordInfo.word, docLang)} style={{ top: wordInfo.top, left: wordInfo.left }} onReplace={replaceWord} /> 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/WordCard.tsx b/web/src/components/Editor/WordCard.tsx index 4649839..a4c6473 100644 --- a/web/src/components/Editor/WordCard.tsx +++ b/web/src/components/Editor/WordCard.tsx @@ -22,11 +22,31 @@ interface Props { // is spelled in letters, and the slashes would say something untrue about it // in the one place a learner is looking for the truth about pronunciation. pinyin?: string + // The locale to pronounce the headword in — the document's language, decided + // by the caller (see docLang in audio/speech.ts). + // + // It has to be passed rather than guessed. `speak` falls back to detecting the + // script, and that test can only tell Han characters from letters: it reads + // "comum" and "casa" as English, so every read-aloud in a Portuguese document + // came out in the English voice. That is the same mistake the "also in" block + // below was built to avoid, arriving through the one button nobody had told + // about the document. + lang?: string style: React.CSSProperties onReplace: (synonym: string) => void } -export function WordCard({ word, info, loading, saved, onToggleSave, pinyin, style, onReplace }: Props) { +export function WordCard({ + word, + info, + loading, + saved, + onToggleSave, + pinyin, + lang, + style, + onReplace, +}: Props) { const t = usePack() const definitions = info?.definitions ?? [] const synonyms = info?.synonyms ?? [] @@ -90,7 +110,7 @@ export function WordCard({ word, info, loading, saved, onToggleSave, pinyin, sty <> - {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 (
`${title} (cópia)`, garden: 'Jardim de palavras', diff --git a/web/src/index.css b/web/src/index.css index b775398..fe07dab 100644 --- a/web/src/index.css +++ b/web/src/index.css @@ -564,6 +564,64 @@ button, a, input { padding-top: 0.25rem; padding-bottom: 0.25rem; } + + /* The formatting toolbar reaches everything it holds by expanding on hover + (see .petal-toolbar above). A touchscreen never hovers, so that rule never + fired here and the row stayed clipped at `overflow: hidden` for good: on a + 390px phone roughly 750px of it — every heading, both lists, all three + alignments, link, image, table, outline, and both AI passes — could not be + reached at all. The faded edge said "there is more this way" and there was + no way. + + So on a coarse pointer the row does what the pill strip does one line + above it: keeps every control and scrolls sideways, but only itself. + overscroll-behavior stops a swipe that runs out of buttons from dragging + the page of writing along with it, and the scrollbar is hidden because a + half-visible button at the edge is the affordance. The panels that hang off + these buttons are placed in viewport coordinates (see Popover in + Toolbar.tsx), so nothing here clips them. */ + .petal-toolbar { + overflow-x: auto; + overflow-y: hidden; + overscroll-behavior-x: contain; + scrollbar-width: none; + -ms-overflow-style: none; + -webkit-mask-image: none; + mask-image: none; + } + .petal-toolbar::-webkit-scrollbar { + display: none; + } + /* :hover can still be reported on a touchscreen — a tap leaves a lingering + hover state on the last thing touched — and the desktop rule would answer + it by unwrapping the row mid-scroll. Hold the scrolling shape instead. */ + .petal-toolbar:hover, + .petal-toolbar:focus-within { + flex-wrap: nowrap; + overflow-x: auto; + overflow-y: hidden; + } + /* Which edge has more behind it, from the same measurement the pill strip + uses (useScrollEdge → data-edge). A row whose buttons all fit is left + unmasked, so the fade only ever appears when it means something. */ + .petal-toolbar[data-edge='right'] { + -webkit-mask-image: linear-gradient(to right, #000 92%, transparent 100%); + mask-image: linear-gradient(to right, #000 92%, transparent 100%); + } + .petal-toolbar[data-edge='left'] { + -webkit-mask-image: linear-gradient(to left, #000 92%, transparent 100%); + mask-image: linear-gradient(to left, #000 92%, transparent 100%); + } + .petal-toolbar[data-edge='both'] { + -webkit-mask-image: linear-gradient( + to right, + transparent 0%, + #000 8%, + #000 92%, + transparent 100% + ); + mask-image: linear-gradient(to right, transparent 0%, #000 8%, #000 92%, transparent 100%); + } } /* --- Responsive sidebar (narrow screens) ------------------------------------ @@ -604,6 +662,17 @@ button, a, input { z-index: 20; background: rgba(61, 46, 57, 0.18); } + + /* Sit the mascot above the status bar rather than on top of it. + --petal-companion-size bottoms out at 9rem, which is most of a phone's + width, and at `bottom-4` the bottom of that circle lands inside the 2.75rem + status bar — directly over "Hide falling petals", which could not be tapped + at all. The kitten yields to cards and panels (useCardOverlap) but the + status bar is neither: it is always there, so yielding to it would mean + fading forever. Moving up once is the honest fix. */ + .petal-corner { + bottom: calc(2.75rem + 0.5rem); + } } /* Small phones only: see the header in App.tsx for why the wordmark yields.