thomwolf HF Staff Claude Opus 5 (1M context) commited on
Commit
8da137f
Β·
1 Parent(s): 07ad947

Second review round: name the page that actually scrolls

Browse files

From the review of 07ad947.

**The baseline was re-taken while the keyboard was up.** `focusin` captured
unconditionally, so tapping from one field to another β€” search box to composer,
which is exactly the sequence the discriminator asks for β€” took the SHRUNK
viewport as the "before". The shrink then measures zero and a keyboard plainly up
reads as absent. Capture only when there is no baseline; `focusout` already
clears it when focus really leaves.

**The comment named the wrong mechanism.** It credited "the same scroll-into-view
that keeps xterm's pinned helper textarea above the keyboard" β€” but measured at
390x844, html, body, #root, .app, .main, .term-host, .pane-reader and .cxv all
have a scroll range of exactly 0. A scroll-into-view inside this document moves
nothing. The reveal is the PARENT page's, wholly, and the comment now says so:
this document's job is to not fight it by resizing itself against a keyboard it
cannot see.

**And what `focusBaseline` is for now.** Since the estimate went, it sizes
nothing β€” its one consumer is the keyboardLayout label that only ?vvdebug=1
reads. Said in both places, so it is not mistaken for load-bearing, or for dead.

Also fixed the readout's own comment, which still described multiplying by a
constant that no longer exists.

tsc clean, exchanges ok, production build succeeds. The measurement above was
taken in headless Chromium against the extracted mobile CSS chain; it says where
scrolling can happen, not what iOS does with a keyboard. Still needs a phone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

web/src/App.tsx CHANGED
@@ -138,7 +138,9 @@ export default function App() {
138
  const keyboardSignalThreshold = 80;
139
  // The viewport as it was before a field took focus β€” the only thing left
140
  // that needs remembering, because a keyboard is detected as the SHRINK from
141
- // it (hasKeyboardGeometry), not as an absolute height.
 
 
142
  let focusBaseline: ViewportBaseline | null = null;
143
 
144
  const acceptsKeyboardInput = (target: Element | null): target is HTMLElement => {
@@ -183,10 +185,17 @@ export default function App() {
183
  // is wrong in the safe direction is still wrong: the abandoned strip does
184
  // not stay hidden behind the keyboard, because the browser scroll-reveals
185
  // a focused field and drags it back into view. That strip is the blank
186
- // band under the reader's composer. Leaving the viewport alone hands the
187
- // job to the engine that can actually see the keyboard β€” the same
188
- // scroll-into-view that already keeps xterm's pinned helper textarea
189
- // above it.
 
 
 
 
 
 
 
190
  if (hasKeyboardGeometry()) root.dataset.keyboardLayout = 'browser-geometry';
191
  else delete root.dataset.keyboardLayout;
192
  root.style.setProperty('--vvw', `${Math.round(width)}px`);
@@ -222,7 +231,12 @@ export default function App() {
222
  };
223
  const onFocusIn = (event: FocusEvent) => {
224
  const target = event.target instanceof Element ? event.target : null;
225
- if (acceptsKeyboardInput(target)) focusBaseline = captureViewport();
 
 
 
 
 
226
  stabilizeFocus();
227
  };
228
  const onFocusOut = () => {
 
138
  const keyboardSignalThreshold = 80;
139
  // The viewport as it was before a field took focus β€” the only thing left
140
  // that needs remembering, because a keyboard is detected as the SHRINK from
141
+ // it (hasKeyboardGeometry), not as an absolute height. Since the estimate
142
+ // was deleted this feeds no layout at all: its one consumer is the
143
+ // keyboardLayout label, which only ?vvdebug=1 reads.
144
  let focusBaseline: ViewportBaseline | null = null;
145
 
146
  const acceptsKeyboardInput = (target: Element | null): target is HTMLElement => {
 
185
  // is wrong in the safe direction is still wrong: the abandoned strip does
186
  // not stay hidden behind the keyboard, because the browser scroll-reveals
187
  // a focused field and drags it back into view. That strip is the blank
188
+ // band under the reader's composer.
189
+ //
190
+ // What replaces it is the PARENT page's scroll, not ours: measured at
191
+ // 390x844, every box in this document β€” html, body, #root, .app, .main,
192
+ // .term-host, .pane-reader, .cxv β€” has a scroll range of exactly 0, so a
193
+ // scroll-into-view in here moves nothing. The reveal is entirely the
194
+ // embedder's, and this document's job is to not fight it by resizing
195
+ // itself against a keyboard it cannot see.
196
+ //
197
+ // Nothing below sizes anything: with the estimate gone, keyboardLayout is
198
+ // a label for ?vvdebug=1 to read. No CSS matches it.
199
  if (hasKeyboardGeometry()) root.dataset.keyboardLayout = 'browser-geometry';
200
  else delete root.dataset.keyboardLayout;
201
  root.style.setProperty('--vvw', `${Math.round(width)}px`);
 
231
  };
232
  const onFocusIn = (event: FocusEvent) => {
233
  const target = event.target instanceof Element ? event.target : null;
234
+ // Only when there is no baseline yet: tapping from one field straight to
235
+ // another keeps the keyboard up, and re-reading here would take the
236
+ // shrunk viewport as the "before" β€” after which the shrink measures zero
237
+ // and a keyboard that is plainly up reads as absent. focusout clears it
238
+ // when focus really leaves, so this stays fresh without being re-taken.
239
+ if (acceptsKeyboardInput(target) && !focusBaseline) focusBaseline = captureViewport();
240
  stabilizeFocus();
241
  };
242
  const onFocusOut = () => {
web/src/components/ViewportDebug.tsx CHANGED
@@ -24,9 +24,10 @@ export default function ViewportDebug() {
24
  let frame = 0;
25
  let last = '';
26
  const history: string[] = [];
27
- // vv.height while nothing is focused: what App.tsx's focusFallback later
28
- // multiplies by its constant. In an iframe this is the FRAME's height, not
29
- // the screen's β€” the discrepancy that makes the guess wrong.
 
30
  let baseline = window.visualViewport?.height ?? window.innerHeight;
31
 
32
  const num = (n: number) => String(Math.round(n)).padStart(4);
 
24
  let frame = 0;
25
  let last = '';
26
  const history: string[] = [];
27
+ // vv.height while nothing is focused. Nothing multiplies it any more β€” the
28
+ // estimate is gone β€” but it is still the number to compare against once a
29
+ // keyboard is up: unchanged here means this document got no signal at all,
30
+ // and inside an iframe it is the FRAME's height, not the screen's.
31
  let baseline = window.visualViewport?.height ?? window.innerHeight;
32
 
33
  const num = (n: number) => String(Math.round(n)).padStart(4);