Read :active, and keep a field's text inside the field
Two prop-level bugs, both of which rendered perfectly and said the wrong
thing. The tag diff could not see either: a tag being HANDLED is not the
same as a tag reading the props a client actually writes.
frq writes :active and :on-toggled on a checkbutton, which is what
libvidya reads — `props.bool("active")` in its tag table. This read
:checked and fired :on-change, so the TLS tick was permanently empty and
the toggle went nowhere. Both spellings are read now and both events fire,
with :active winning where a client writes both.
And a field's text ran straight out past its own border and over whatever
was beside it. It is clipped to the field now and slides left to keep the
caret in view, which a handle, a URL or a password all need — a field
narrower than its contents is the ordinary case, not the exceptional one.
The caret rides the same shift and the same clip; drawn at the
untranslated offset it sat outside the border pointing at where the text
would have been.
Checked by pixel, because both of these looked fine: the tick must put ink
in its little box, and nothing may be drawn past the field's right edge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>c131fc6 parent: 58ad86a modified
glimmer-backends/glimmer-jvui/src/glimmer_jvui/core.clj +18 -5 | @@ -187,11 +187,24 @@ | ||
| 187 | 187 | ;; :checkbutton is the same widget under GTK's name for it, which is |
| 188 | 188 | ;; what libvidya calls it too — `"checkbutton" | "checkbox"` is one |
| 189 | 189 | ;; arm of its tag table. frq writes both. |
| 190 | - (:checkbox :checkbutton) (let [was (boolean (:checked props)) | |
| 191 | - id (c/next-id key) | |
| 192 | - now (w/checkbox was s {:key key})] | |
| 193 | - (record! n id) | |
| 194 | - (when (not= now was) (fire! n :on-change now))) | |
| 190 | + ;; :active is what frq and libvidya call it — `props.bool("active")` | |
| 191 | + ;; in libvidya's tag table — and :checked is what this backend called | |
| 192 | + ;; it first. Both are read, because a client written against either | |
| 193 | + ;; should not render a permanently empty tick; :active wins where | |
| 194 | + ;; both appear. | |
| 195 | + ;; | |
| 196 | + ;; Likewise both events fire. libvidya emits "toggled"; :on-change is | |
| 197 | + ;; what the checkbox here answered to before. | |
| 198 | + (:checkbox :checkbutton) | |
| 199 | + (let [was (boolean (if (contains? props :active) | |
| 200 | + (:active props) | |
| 201 | + (:checked props))) | |
| 202 | + id (c/next-id key) | |
| 203 | + now (w/checkbox was s {:key key})] | |
| 204 | + (record! n id) | |
| 205 | + (when (not= now was) | |
| 206 | + (fire! n :on-toggled now) | |
| 207 | + (fire! n :on-change now))) | |
| 195 | 208 | |
| 196 | 209 | :slider (let [was (num (:value props) 0.0) |
| 197 | 210 | id (c/next-id key) |
| @@ -187,11 +187,24 @@ | |||
| 187 | ;; :checkbutton is the same widget under GTK's name for it, which is | 187 | ;; :checkbutton is the same widget under GTK's name for it, which is |
| 188 | ;; what libvidya calls it too — `"checkbutton" | "checkbox"` is one | 188 | ;; what libvidya calls it too — `"checkbutton" | "checkbox"` is one |
| 189 | ;; arm of its tag table. frq writes both. | 189 | ;; arm of its tag table. frq writes both. |
| 190 | - (:checkbox :checkbutton) (let [was (boolean (:checked props)) | 190 | + ;; :active is what frq and libvidya call it — `props.bool("active")` |
| 191 | - id (c/next-id key) | 191 | + ;; in libvidya's tag table — and :checked is what this backend called |
| 192 | - now (w/checkbox was s {:key key})] | 192 | + ;; it first. Both are read, because a client written against either |
| 193 | - (record! n id) | 193 | + ;; should not render a permanently empty tick; :active wins where |
| 194 | - (when (not= now was) (fire! n :on-change now))) | 194 | + ;; both appear. |
| 195 | + ;; | ||
| 196 | + ;; Likewise both events fire. libvidya emits "toggled"; :on-change is | ||
| 197 | + ;; what the checkbox here answered to before. | ||
| 198 | + (:checkbox :checkbutton) | ||
| 199 | + (let [was (boolean (if (contains? props :active) | ||
| 200 | + (:active props) | ||
| 201 | + (:checked props))) | ||
| 202 | + id (c/next-id key) | ||
| 203 | + now (w/checkbox was s {:key key})] | ||
| 204 | + (record! n id) | ||
| 205 | + (when (not= now was) | ||
| 206 | + (fire! n :on-toggled now) | ||
| 207 | + (fire! n :on-change now))) | ||
| 195 | 208 | ||
| 196 | :slider (let [was (num (:value props) 0.0) | 209 | :slider (let [was (num (:value props) 0.0) |
| 197 | id (c/next-id key) | 210 | id (c/next-id key) |
added
glimmer-backends/glimmer-jvui/test/glimmer_jvui/props_check.clj +55 -0 | new file mode 100644 | ||
| @@ -0,0 +1,55 @@ | ||
| 1 | +(ns glimmer-jvui.props-check | |
| 2 | + "Two prop-level bugs the tag diff could not see, checked against pixels. | |
| 3 | + | |
| 4 | + A tag being handled is not the same as a tag reading the props a client | |
| 5 | + actually writes. Both of these rendered perfectly and said the wrong | |
| 6 | + thing: a checkbutton whose tick never appeared because frq writes | |
| 7 | + `:active` where this read `:checked`, and a field whose text ran out past | |
| 8 | + its own border because nothing clipped it." | |
| 9 | + (:require [glimmer.core :as ui] | |
| 10 | + [glimmer-jvui.core] | |
| 11 | + [jvui.sdl :as sdl] | |
| 12 | + [jolt.ffi :as ffi])) | |
| 13 | + | |
| 14 | +(defn- bmp [path] | |
| 15 | + (let [b (java.nio.file.Files/readAllBytes | |
| 16 | + (java.nio.file.Path/of path (into-array String []))) | |
| 17 | + u (fn [i] (bit-and (int (aget b i)) 255)) | |
| 18 | + le (fn [i] (+ (u i) (bit-shift-left (u (+ i 1)) 8) | |
| 19 | + (bit-shift-left (u (+ i 2)) 16) (bit-shift-left (u (+ i 3)) 24))) | |
| 20 | + off (le 10) w (le 18) h (le 22) stride (* 4 (quot (+ (* w 3) 3) 4))] | |
| 21 | + {:w w :h h :px (fn [x y] (let [i (+ off (* (- h 1 y) stride) (* 3 x))] | |
| 22 | + {:b (u i) :g (u (+ i 1)) :r (u (+ i 2))}))})) | |
| 23 | + | |
| 24 | +(defn- ink-in | |
| 25 | + "How many pixels inside [x0 y0 x1 y1] differ from the background." | |
| 26 | + [{:keys [px]} bg x0 y0 x1 y1] | |
| 27 | + (count (for [y (range y0 y1) x (range x0 x1) | |
| 28 | + :let [p (px x y)] | |
| 29 | + :when (> (+ (abs (- (:r p) (:r bg))) (abs (- (:g p) (:g bg))) | |
| 30 | + (abs (- (:b p) (:b bg)))) 40)] | |
| 31 | + 1))) | |
| 32 | + | |
| 33 | +(defn -main [& _] | |
| 34 | + (let [shot (str (System/getProperty "java.io.tmpdir") "/jvui-props.bmp") | |
| 35 | + out (atom []) ck! (fn [n ok?] (swap! out conj [n (boolean ok?)]))] | |
| 36 | + (ui/run (fn [] | |
| 37 | + [:vbox {:spacing 8} | |
| 38 | + ;; :active, the way frq writes it — the tick must appear. | |
| 39 | + [:checkbutton {:label "TLS" :active true}] | |
| 40 | + ;; A string far wider than the field it is in. | |
| 41 | + [:entry {:value "nandi-test.bsky.social-and-then-some-more-text"}]]) | |
| 42 | + {:title "props" :width 240 :height 140 :frames 6 :shot shot}) | |
| 43 | + (let [img (bmp shot) | |
| 44 | + bg ((:px img) 2 2)] | |
| 45 | + ;; The tick is inside the little box at the far left of the first row. | |
| 46 | + (ck! "an :active checkbutton draws its tick" | |
| 47 | + (> (ink-in img bg 4 4 26 30) 12)) | |
| 48 | + ;; Nothing may be drawn to the right of the field's own border. | |
| 49 | + (ck! "entry text stops at the field's edge" | |
| 50 | + (zero? (ink-in img bg 232 40 240 100)))) | |
| 51 | + (doseq [[n ok?] @out] (println (if ok? "- " "FAIL ") n)) | |
| 52 | + (let [bad (remove second @out)] | |
| 53 | + (println (if (seq bad) (str (count bad) " of " (count @out) " checks FAILED") | |
| 54 | + (str "all " (count @out) " checks passed"))) | |
| 55 | + (when (seq bad) (System/exit 1))))) | |
| new file mode 100644 | |||
| @@ -0,0 +1,55 @@ | |||
| 1 | +(ns glimmer-jvui.props-check | ||
| 2 | + "Two prop-level bugs the tag diff could not see, checked against pixels. | ||
| 3 | + | ||
| 4 | + A tag being handled is not the same as a tag reading the props a client | ||
| 5 | + actually writes. Both of these rendered perfectly and said the wrong | ||
| 6 | + thing: a checkbutton whose tick never appeared because frq writes | ||
| 7 | + `:active` where this read `:checked`, and a field whose text ran out past | ||
| 8 | + its own border because nothing clipped it." | ||
| 9 | + (:require [glimmer.core :as ui] | ||
| 10 | + [glimmer-jvui.core] | ||
| 11 | + [jvui.sdl :as sdl] | ||
| 12 | + [jolt.ffi :as ffi])) | ||
| 13 | + | ||
| 14 | +(defn- bmp [path] | ||
| 15 | + (let [b (java.nio.file.Files/readAllBytes | ||
| 16 | + (java.nio.file.Path/of path (into-array String []))) | ||
| 17 | + u (fn [i] (bit-and (int (aget b i)) 255)) | ||
| 18 | + le (fn [i] (+ (u i) (bit-shift-left (u (+ i 1)) 8) | ||
| 19 | + (bit-shift-left (u (+ i 2)) 16) (bit-shift-left (u (+ i 3)) 24))) | ||
| 20 | + off (le 10) w (le 18) h (le 22) stride (* 4 (quot (+ (* w 3) 3) 4))] | ||
| 21 | + {:w w :h h :px (fn [x y] (let [i (+ off (* (- h 1 y) stride) (* 3 x))] | ||
| 22 | + {:b (u i) :g (u (+ i 1)) :r (u (+ i 2))}))})) | ||
| 23 | + | ||
| 24 | +(defn- ink-in | ||
| 25 | + "How many pixels inside [x0 y0 x1 y1] differ from the background." | ||
| 26 | + [{:keys [px]} bg x0 y0 x1 y1] | ||
| 27 | + (count (for [y (range y0 y1) x (range x0 x1) | ||
| 28 | + :let [p (px x y)] | ||
| 29 | + :when (> (+ (abs (- (:r p) (:r bg))) (abs (- (:g p) (:g bg))) | ||
| 30 | + (abs (- (:b p) (:b bg)))) 40)] | ||
| 31 | + 1))) | ||
| 32 | + | ||
| 33 | +(defn -main [& _] | ||
| 34 | + (let [shot (str (System/getProperty "java.io.tmpdir") "/jvui-props.bmp") | ||
| 35 | + out (atom []) ck! (fn [n ok?] (swap! out conj [n (boolean ok?)]))] | ||
| 36 | + (ui/run (fn [] | ||
| 37 | + [:vbox {:spacing 8} | ||
| 38 | + ;; :active, the way frq writes it — the tick must appear. | ||
| 39 | + [:checkbutton {:label "TLS" :active true}] | ||
| 40 | + ;; A string far wider than the field it is in. | ||
| 41 | + [:entry {:value "nandi-test.bsky.social-and-then-some-more-text"}]]) | ||
| 42 | + {:title "props" :width 240 :height 140 :frames 6 :shot shot}) | ||
| 43 | + (let [img (bmp shot) | ||
| 44 | + bg ((:px img) 2 2)] | ||
| 45 | + ;; The tick is inside the little box at the far left of the first row. | ||
| 46 | + (ck! "an :active checkbutton draws its tick" | ||
| 47 | + (> (ink-in img bg 4 4 26 30) 12)) | ||
| 48 | + ;; Nothing may be drawn to the right of the field's own border. | ||
| 49 | + (ck! "entry text stops at the field's edge" | ||
| 50 | + (zero? (ink-in img bg 232 40 240 100)))) | ||
| 51 | + (doseq [[n ok?] @out] (println (if ok? "- " "FAIL ") n)) | ||
| 52 | + (let [bad (remove second @out)] | ||
| 53 | + (println (if (seq bad) (str (count bad) " of " (count @out) " checks FAILED") | ||
| 54 | + (str "all " (count @out) " checks passed"))) | ||
| 55 | + (when (seq bad) (System/exit 1))))) | ||
modified
jvui/src/jvui/core.clj +11 -0 | @@ -296,6 +296,17 @@ | ||
| 296 | 296 | (upd! assoc :clip rect) |
| 297 | 297 | (when (drawing?) (paint/set-clip! (:painter (ui)) rect))) |
| 298 | 298 | |
| 299 | +(defn with-clip | |
| 300 | + "Run `f` with drawing confined to `rect`, then put the old clip back. | |
| 301 | + | |
| 302 | + Intersected with whatever clip is already in force, so a field inside a | |
| 303 | + scrolled viewport is bounded by both and not just by itself." | |
| 304 | + [rect f] | |
| 305 | + (let [prev (:clip (ui)) | |
| 306 | + r (if prev (intersect prev rect) rect)] | |
| 307 | + (set-clip! r) | |
| 308 | + (try (f) (finally (set-clip! prev))))) | |
| 309 | + | |
| 299 | 310 | (defn box* |
| 300 | 311 | "The container everything else is built from. |
| 301 | 312 | |
| @@ -296,6 +296,17 @@ | |||
| 296 | (upd! assoc :clip rect) | 296 | (upd! assoc :clip rect) |
| 297 | (when (drawing?) (paint/set-clip! (:painter (ui)) rect))) | 297 | (when (drawing?) (paint/set-clip! (:painter (ui)) rect))) |
| 298 | 298 | ||
| 299 | +(defn with-clip | ||
| 300 | + "Run `f` with drawing confined to `rect`, then put the old clip back. | ||
| 301 | + | ||
| 302 | + Intersected with whatever clip is already in force, so a field inside a | ||
| 303 | + scrolled viewport is bounded by both and not just by itself." | ||
| 304 | + [rect f] | ||
| 305 | + (let [prev (:clip (ui)) | ||
| 306 | + r (if prev (intersect prev rect) rect)] | ||
| 307 | + (set-clip! r) | ||
| 308 | + (try (f) (finally (set-clip! prev))))) | ||
| 309 | + | ||
| 299 | (defn box* | 310 | (defn box* |
| 300 | "The container everything else is built from. | 311 | "The container everything else is built from. |
| 301 | 312 | ||
modified
jvui/src/jvui/widgets.clj +20 -6 | @@ -323,13 +323,27 @@ | ||
| 323 | 323 | (if focused? (c/th :focus) (c/th :border)) |
| 324 | 324 | (if focused? 2.0 (c/th :border-width))) |
| 325 | 325 | (let [[_ th*] (c/measure (if (= s' "") "M" s') size) |
| 326 | - ty (+ ry (/ (- rh th*) 2.0))] | |
| 327 | - (if (and (= s' "") placeholder (not focused?)) | |
| 328 | - (c/draw-text! placeholder (+ rx pad) ty size (c/th :text-dim)) | |
| 329 | - (c/draw-text! s' (+ rx pad) ty size (c/th :text))) | |
| 326 | + ty (+ ry (/ (- rh th*) 2.0)) | |
| 327 | + inner (- rw (* 2 pad)) | |
| 328 | + ;; How far the text is slid left so the caret stays in view. A | |
| 329 | + ;; field narrower than its contents is the ordinary case — a | |
| 330 | + ;; handle, a URL, a password — and without this the text simply | |
| 331 | + ;; runs out past the border and over whatever is beside it. | |
| 332 | + caret-x (first (c/measure (subs s' 0 caret') size)) | |
| 333 | + shift (max 0.0 (- caret-x inner))] | |
| 334 | + (c/with-clip [(+ rx pad) ry inner rh] | |
| 335 | + (fn [] | |
| 336 | + (if (and (= s' "") placeholder (not focused?)) | |
| 337 | + (c/draw-text! placeholder (+ rx pad) ty size (c/th :text-dim)) | |
| 338 | + (c/draw-text! s' (- (+ rx pad) shift) ty size (c/th :text))))) | |
| 339 | + ;; The caret rides the same shift, and inside the same clip: a caret | |
| 340 | + ;; drawn at the untranslated offset sits past the border on a full | |
| 341 | + ;; field, pointing at where the text would have been. | |
| 330 | 342 | (when focused? |
| 331 | - (let [[cw _] (c/measure (subs s' 0 caret') size)] | |
| 332 | - (c/fill! [(+ rx pad cw) (+ ty 1.0) 1.5 (- th* 2.0)] (c/th :text))))) | |
| 343 | + (c/with-clip [(+ rx pad) ry inner rh] | |
| 344 | + (fn [] | |
| 345 | + (c/fill! [(- (+ rx pad caret-x) shift) (+ ty 1.0) 1.5 (- th* 2.0)] | |
| 346 | + (c/th :text)))))) | |
| 333 | 347 | s'))) |
| 334 | 348 | |
| 335 | 349 | ;; ------------------------------------------------------------------- scroll |
| @@ -323,13 +323,27 @@ | |||
| 323 | (if focused? (c/th :focus) (c/th :border)) | 323 | (if focused? (c/th :focus) (c/th :border)) |
| 324 | (if focused? 2.0 (c/th :border-width))) | 324 | (if focused? 2.0 (c/th :border-width))) |
| 325 | (let [[_ th*] (c/measure (if (= s' "") "M" s') size) | 325 | (let [[_ th*] (c/measure (if (= s' "") "M" s') size) |
| 326 | - ty (+ ry (/ (- rh th*) 2.0))] | 326 | + ty (+ ry (/ (- rh th*) 2.0)) |
| 327 | - (if (and (= s' "") placeholder (not focused?)) | 327 | + inner (- rw (* 2 pad)) |
| 328 | - (c/draw-text! placeholder (+ rx pad) ty size (c/th :text-dim)) | 328 | + ;; How far the text is slid left so the caret stays in view. A |
| 329 | - (c/draw-text! s' (+ rx pad) ty size (c/th :text))) | 329 | + ;; field narrower than its contents is the ordinary case — a |
| 330 | + ;; handle, a URL, a password — and without this the text simply | ||
| 331 | + ;; runs out past the border and over whatever is beside it. | ||
| 332 | + caret-x (first (c/measure (subs s' 0 caret') size)) | ||
| 333 | + shift (max 0.0 (- caret-x inner))] | ||
| 334 | + (c/with-clip [(+ rx pad) ry inner rh] | ||
| 335 | + (fn [] | ||
| 336 | + (if (and (= s' "") placeholder (not focused?)) | ||
| 337 | + (c/draw-text! placeholder (+ rx pad) ty size (c/th :text-dim)) | ||
| 338 | + (c/draw-text! s' (- (+ rx pad) shift) ty size (c/th :text))))) | ||
| 339 | + ;; The caret rides the same shift, and inside the same clip: a caret | ||
| 340 | + ;; drawn at the untranslated offset sits past the border on a full | ||
| 341 | + ;; field, pointing at where the text would have been. | ||
| 330 | (when focused? | 342 | (when focused? |
| 331 | - (let [[cw _] (c/measure (subs s' 0 caret') size)] | 343 | + (c/with-clip [(+ rx pad) ry inner rh] |
| 332 | - (c/fill! [(+ rx pad cw) (+ ty 1.0) 1.5 (- th* 2.0)] (c/th :text))))) | 344 | + (fn [] |
| 345 | + (c/fill! [(- (+ rx pad caret-x) shift) (+ ty 1.0) 1.5 (- th* 2.0)] | ||
| 346 | + (c/th :text)))))) | ||
| 333 | s'))) | 347 | s'))) |
| 334 | 348 | ||
| 335 | ;; ------------------------------------------------------------------- scroll | 349 | ;; ------------------------------------------------------------------- scroll |