Fire :on-activate with nothing, wrap a row, and break a word that cannot fit
Three things running frq turned up, two of them mine from the last commit. :on-activate was fired WITH the field's text and frq's handlers are thunks — s/send-draft! takes none, and the join field's is an anonymous zero-arg fn. So the first Enter anyone pressed was an arity error and the window went with it. libvidya emits activate with an empty string for the same reason: the text is already the caller's, they had it from :on-change. Fired with nothing now, and the test's own handler was written the wrong way round too, which is why it passed. A word that cannot fit on a line is broken at a character now. The first version left it long on the grounds that a URL cut in half reads worse than one that overflows. That was wrong in its consequence and the sign-in screen showed it: an OAuth login URL is one unbreakable word, so it made its container wider than the window and pushed the whole page off both edges. A broken URL is worse than an unbroken one; a window you cannot read is worse than both. And :wrap on a row, which is what a line of reaction pills needs. A wrapped row measures differently in both directions — its own axis is the widest row rather than the run of every child, and its cross axis is the rows stacked rather than the tallest child — so it is a change to how a box counts and not only to where it puts things. A child too wide for the whole box is never wrapped: something has to go somewhere, and wrapping it forever is worse than letting it overflow. :align comes along with it, as a cross-axis gravity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2271a91 parent: 95540ef modified
glimmer-backends/glimmer-jvui/src/glimmer_jvui/core.clj +16 -2 | @@ -78,7 +78,15 @@ | ||
| 78 | 78 | (:spacing props) (assoc :spacing (num (:spacing props) 0.0)) |
| 79 | 79 | (:padding props) (assoc :padding (num (:padding props) 0.0)) |
| 80 | 80 | (:margin props) (assoc :margin (num (:margin props) 0.0)) |
| 81 | - (:expand props) (assoc :expand (:expand props)))) | |
| 81 | + (:expand props) (assoc :expand (:expand props)) | |
| 82 | + ;; A row whose children start a new line when they run out of room — | |
| 83 | + ;; a line of reaction pills is the case that needs it. | |
| 84 | + (:wrap props) (assoc :wrap true) | |
| 85 | + ;; Cross-axis placement: :start :center :end, as a gravity. | |
| 86 | + (:align props) (assoc :gravity (case (:align props) | |
| 87 | + (:center "center") [0.0 0.5] | |
| 88 | + (:end "end") [0.0 1.0] | |
| 89 | + [0.0 0.0])))) | |
| 82 | 90 | |
| 83 | 91 | (defn- fire! [n k & args] |
| 84 | 92 | (when-let [f (get (:props @n) k)] (apply f args))) |
| @@ -254,7 +262,13 @@ | ||
| 254 | 262 | ;; Enter, which a field must not swallow as input: frq sends its |
| 255 | 263 | ;; message on it, and without this the compose box accepted text |
| 256 | 264 | ;; and had no way to say it was finished. |
| 257 | - (when (w/entry-activated? id) (fire! n :on-activate now))) | |
| 265 | + ;; | |
| 266 | + ;; NO ARGUMENT. libvidya emits activate with an empty string, and | |
| 267 | + ;; frq's handlers are thunks — `s/send-draft!` takes none, and | |
| 268 | + ;; handing it the text is an arity error the moment somebody | |
| 269 | + ;; presses Enter. The text is already theirs; they got it from | |
| 270 | + ;; :on-change. | |
| 271 | + (when (w/entry-activated? id) (fire! n :on-activate))) | |
| 258 | 272 | |
| 259 | 273 | :progress (w/progress (num (:value props) 0.0)) |
| 260 | 274 | :separator (w/separator) |
| @@ -78,7 +78,15 @@ | |||
| 78 | (:spacing props) (assoc :spacing (num (:spacing props) 0.0)) | 78 | (:spacing props) (assoc :spacing (num (:spacing props) 0.0)) |
| 79 | (:padding props) (assoc :padding (num (:padding props) 0.0)) | 79 | (:padding props) (assoc :padding (num (:padding props) 0.0)) |
| 80 | (:margin props) (assoc :margin (num (:margin props) 0.0)) | 80 | (:margin props) (assoc :margin (num (:margin props) 0.0)) |
| 81 | - (:expand props) (assoc :expand (:expand props)))) | 81 | + (:expand props) (assoc :expand (:expand props)) |
| 82 | + ;; A row whose children start a new line when they run out of room — | ||
| 83 | + ;; a line of reaction pills is the case that needs it. | ||
| 84 | + (:wrap props) (assoc :wrap true) | ||
| 85 | + ;; Cross-axis placement: :start :center :end, as a gravity. | ||
| 86 | + (:align props) (assoc :gravity (case (:align props) | ||
| 87 | + (:center "center") [0.0 0.5] | ||
| 88 | + (:end "end") [0.0 1.0] | ||
| 89 | + [0.0 0.0])))) | ||
| 82 | 90 | ||
| 83 | (defn- fire! [n k & args] | 91 | (defn- fire! [n k & args] |
| 84 | (when-let [f (get (:props @n) k)] (apply f args))) | 92 | (when-let [f (get (:props @n) k)] (apply f args))) |
| @@ -254,7 +262,13 @@ | |||
| 254 | ;; Enter, which a field must not swallow as input: frq sends its | 262 | ;; Enter, which a field must not swallow as input: frq sends its |
| 255 | ;; message on it, and without this the compose box accepted text | 263 | ;; message on it, and without this the compose box accepted text |
| 256 | ;; and had no way to say it was finished. | 264 | ;; and had no way to say it was finished. |
| 257 | - (when (w/entry-activated? id) (fire! n :on-activate now))) | 265 | + ;; |
| 266 | + ;; NO ARGUMENT. libvidya emits activate with an empty string, and | ||
| 267 | + ;; frq's handlers are thunks — `s/send-draft!` takes none, and | ||
| 268 | + ;; handing it the text is an arity error the moment somebody | ||
| 269 | + ;; presses Enter. The text is already theirs; they got it from | ||
| 270 | + ;; :on-change. | ||
| 271 | + (when (w/entry-activated? id) (fire! n :on-activate))) | ||
| 258 | 272 | ||
| 259 | :progress (w/progress (num (:value props) 0.0)) | 273 | :progress (w/progress (num (:value props) 0.0)) |
| 260 | :separator (w/separator) | 274 | :separator (w/separator) |
modified
glimmer-backends/glimmer-jvui/test/glimmer_jvui/tests.clj +5 -1 | @@ -161,7 +161,11 @@ | ||
| 161 | 161 | (gui/mount root :page |
| 162 | 162 | [(fn [] [:entry {:value (ra/deref text) |
| 163 | 163 | :on-change #(ra/reset! text %) |
| 164 | - :on-activate #(ra/reset! sent %)}])]) | |
| 164 | + ;; A THUNK, which is what frq's handlers | |
| 165 | + ;; are: s/send-draft! takes no arguments | |
| 166 | + ;; and the text is already the caller's | |
| 167 | + ;; from :on-change. | |
| 168 | + :on-activate #(ra/reset! sent (ra/deref text))}])]) | |
| 165 | 169 | (jv/render-once root cx) |
| 166 | 170 | (let [[ex ey] (centre (tagged root :entry))] |
| 167 | 171 | (jv/render-once root cx (click-at ex ey))) ; take focus |
| @@ -161,7 +161,11 @@ | |||
| 161 | (gui/mount root :page | 161 | (gui/mount root :page |
| 162 | [(fn [] [:entry {:value (ra/deref text) | 162 | [(fn [] [:entry {:value (ra/deref text) |
| 163 | :on-change #(ra/reset! text %) | 163 | :on-change #(ra/reset! text %) |
| 164 | - :on-activate #(ra/reset! sent %)}])]) | 164 | + ;; A THUNK, which is what frq's handlers |
| 165 | + ;; are: s/send-draft! takes no arguments | ||
| 166 | + ;; and the text is already the caller's | ||
| 167 | + ;; from :on-change. | ||
| 168 | + :on-activate #(ra/reset! sent (ra/deref text))}])]) | ||
| 165 | (jv/render-once root cx) | 169 | (jv/render-once root cx) |
| 166 | (let [[ex ey] (centre (tagged root :entry))] | 170 | (let [[ex ey] (centre (tagged root :entry))] |
| 167 | (jv/render-once root cx (click-at ex ey))) ; take focus | 171 | (jv/render-once root cx (click-at ex ey))) ; take focus |
modified
jvui/src/jvui/core.clj +36 -6 | @@ -195,8 +195,13 @@ | ||
| 195 | 195 | (def ^:private CROSS 2) |
| 196 | 196 | (def ^:private N 3) |
| 197 | 197 | (def ^:private EXPANDERS 4) |
| 198 | +;; Wrapped rows only: where the current row starts, and how tall it is so | |
| 199 | +;; far. A box that does not wrap leaves both at zero and behaves exactly as | |
| 200 | +;; it did. | |
| 201 | +(def ^:private ROWY 5) | |
| 202 | +(def ^:private ROWH 6) | |
| 198 | 203 | |
| 199 | -(defn- counters [] (double-array 5)) | |
| 204 | +(defn- counters [] (double-array 7)) | |
| 200 | 205 | (defn- ctr [f i] (aget (:c f) i)) |
| 201 | 206 | (defn- ctr+ [f i v] (aset (:c f) i (+ (aget (:c f) i) v))) |
| 202 | 207 | |
| @@ -217,8 +222,17 @@ | ||
| 217 | 222 | [advance min-along min-cross grew?] |
| 218 | 223 | (let [f (top)] |
| 219 | 224 | (ctr+ f CURSOR (+ advance (:spacing f))) |
| 220 | - (ctr+ f ALONG (+ min-along (if (pos? (ctr f N)) (:spacing f) 0.0))) | |
| 221 | - (aset (:c f) CROSS (max (ctr f CROSS) (double min-cross))) | |
| 225 | + (if (:wrap f) | |
| 226 | + ;; A wrapped row measures differently in both directions: its own | |
| 227 | + ;; axis is the WIDEST row rather than the run of every child, and | |
| 228 | + ;; its cross axis is the rows stacked rather than the tallest child. | |
| 229 | + (do | |
| 230 | + (aset (:c f) ALONG (max (ctr f ALONG) (- (ctr f CURSOR) (:spacing f)))) | |
| 231 | + (aset (:c f) ROWH (max (ctr f ROWH) (double min-cross))) | |
| 232 | + (aset (:c f) CROSS (+ (ctr f ROWY) (ctr f ROWH)))) | |
| 233 | + (do | |
| 234 | + (ctr+ f ALONG (+ min-along (if (pos? (ctr f N)) (:spacing f) 0.0))) | |
| 235 | + (aset (:c f) CROSS (max (ctr f CROSS) (double min-cross))))) | |
| 222 | 236 | (ctr+ f N 1.0) |
| 223 | 237 | (when grew? (ctr+ f EXPANDERS 1.0)))) |
| 224 | 238 | |
| @@ -250,9 +264,18 @@ | ||
| 250 | 264 | (tally! h mh mw grow-along?) |
| 251 | 265 | [x y w h]) |
| 252 | 266 | (let [w (+ mw (if grow-along? share 0.0)) |
| 267 | + ;; Wrap BEFORE placing: a child that will not fit on this row | |
| 268 | + ;; starts the next one. Never on the first child of a row — | |
| 269 | + ;; something wider than the whole box has to go somewhere, and | |
| 270 | + ;; wrapping it forever is worse than letting it overflow. | |
| 271 | + _ (when (and (:wrap f) (pos? (ctr f N)) (> (+ cursor w) rw)) | |
| 272 | + (aset (:c f) CURSOR 0.0) | |
| 273 | + (ctr+ f ROWY (+ (ctr f ROWH) (:spacing f))) | |
| 274 | + (aset (:c f) ROWH 0.0)) | |
| 275 | + cursor (ctr f CURSOR) | |
| 253 | 276 | h (if grow-cross? rh mh) |
| 254 | 277 | x (+ rx ox cursor) |
| 255 | - y (+ ry oy (* gy (- rh h)))] | |
| 278 | + y (+ ry oy (ctr f ROWY) (if (:wrap f) 0.0 (* gy (- rh h))))] | |
| 256 | 279 | (tally! w mw mh grow-along?) |
| 257 | 280 | [x y w h])))) |
| 258 | 281 | |
| @@ -326,10 +349,14 @@ | ||
| 326 | 349 | "The container everything else is built from. |
| 327 | 350 | |
| 328 | 351 | opts: :dir :spacing :padding :margin :expand :gravity :key :fill :border |
| 329 | - :radius :min-size :clip? :offset" | |
| 352 | + :radius :min-size :clip? :offset :wrap | |
| 353 | + | |
| 354 | + :wrap only means anything on a row: children that will not fit start a | |
| 355 | + new line instead of running off the end. A column already puts every | |
| 356 | + child on a line of its own." | |
| 330 | 357 | [opts body] |
| 331 | 358 | (let [{:keys [dir spacing padding margin expand gravity key fill border |
| 332 | - radius min-size clip? offset fixed] | |
| 359 | + radius min-size clip? offset fixed wrap] | |
| 333 | 360 | :or {dir :vertical expand :none gravity [0.0 0.0] clip? false}} opts |
| 334 | 361 | spacing (double (or spacing 0.0)) |
| 335 | 362 | padding (double (or padding 0.0)) |
| @@ -371,6 +398,9 @@ | ||
| 371 | 398 | (when clip? (set-clip! (intersect (or outer-clip content) content))) |
| 372 | 399 | (push-frame! {:id id :dir dir :rect content :spacing spacing |
| 373 | 400 | :share share :c (counters) |
| 401 | + ;; Only a row can wrap. A column already puts every child | |
| 402 | + ;; on a line of its own. | |
| 403 | + :wrap (boolean (and wrap (= dir :horizontal))) | |
| 374 | 404 | :offset (or offset [0.0 0.0])}) |
| 375 | 405 | (let [result (when body (body id box-rect)) |
| 376 | 406 | f (pop-frame!)] |
| @@ -195,8 +195,13 @@ | |||
| 195 | (def ^:private CROSS 2) | 195 | (def ^:private CROSS 2) |
| 196 | (def ^:private N 3) | 196 | (def ^:private N 3) |
| 197 | (def ^:private EXPANDERS 4) | 197 | (def ^:private EXPANDERS 4) |
| 198 | +;; Wrapped rows only: where the current row starts, and how tall it is so | ||
| 199 | +;; far. A box that does not wrap leaves both at zero and behaves exactly as | ||
| 200 | +;; it did. | ||
| 201 | +(def ^:private ROWY 5) | ||
| 202 | +(def ^:private ROWH 6) | ||
| 198 | 203 | ||
| 199 | -(defn- counters [] (double-array 5)) | 204 | +(defn- counters [] (double-array 7)) |
| 200 | (defn- ctr [f i] (aget (:c f) i)) | 205 | (defn- ctr [f i] (aget (:c f) i)) |
| 201 | (defn- ctr+ [f i v] (aset (:c f) i (+ (aget (:c f) i) v))) | 206 | (defn- ctr+ [f i v] (aset (:c f) i (+ (aget (:c f) i) v))) |
| 202 | 207 | ||
| @@ -217,8 +222,17 @@ | |||
| 217 | [advance min-along min-cross grew?] | 222 | [advance min-along min-cross grew?] |
| 218 | (let [f (top)] | 223 | (let [f (top)] |
| 219 | (ctr+ f CURSOR (+ advance (:spacing f))) | 224 | (ctr+ f CURSOR (+ advance (:spacing f))) |
| 220 | - (ctr+ f ALONG (+ min-along (if (pos? (ctr f N)) (:spacing f) 0.0))) | 225 | + (if (:wrap f) |
| 221 | - (aset (:c f) CROSS (max (ctr f CROSS) (double min-cross))) | 226 | + ;; A wrapped row measures differently in both directions: its own |
| 227 | + ;; axis is the WIDEST row rather than the run of every child, and | ||
| 228 | + ;; its cross axis is the rows stacked rather than the tallest child. | ||
| 229 | + (do | ||
| 230 | + (aset (:c f) ALONG (max (ctr f ALONG) (- (ctr f CURSOR) (:spacing f)))) | ||
| 231 | + (aset (:c f) ROWH (max (ctr f ROWH) (double min-cross))) | ||
| 232 | + (aset (:c f) CROSS (+ (ctr f ROWY) (ctr f ROWH)))) | ||
| 233 | + (do | ||
| 234 | + (ctr+ f ALONG (+ min-along (if (pos? (ctr f N)) (:spacing f) 0.0))) | ||
| 235 | + (aset (:c f) CROSS (max (ctr f CROSS) (double min-cross))))) | ||
| 222 | (ctr+ f N 1.0) | 236 | (ctr+ f N 1.0) |
| 223 | (when grew? (ctr+ f EXPANDERS 1.0)))) | 237 | (when grew? (ctr+ f EXPANDERS 1.0)))) |
| 224 | 238 | ||
| @@ -250,9 +264,18 @@ | |||
| 250 | (tally! h mh mw grow-along?) | 264 | (tally! h mh mw grow-along?) |
| 251 | [x y w h]) | 265 | [x y w h]) |
| 252 | (let [w (+ mw (if grow-along? share 0.0)) | 266 | (let [w (+ mw (if grow-along? share 0.0)) |
| 267 | + ;; Wrap BEFORE placing: a child that will not fit on this row | ||
| 268 | + ;; starts the next one. Never on the first child of a row — | ||
| 269 | + ;; something wider than the whole box has to go somewhere, and | ||
| 270 | + ;; wrapping it forever is worse than letting it overflow. | ||
| 271 | + _ (when (and (:wrap f) (pos? (ctr f N)) (> (+ cursor w) rw)) | ||
| 272 | + (aset (:c f) CURSOR 0.0) | ||
| 273 | + (ctr+ f ROWY (+ (ctr f ROWH) (:spacing f))) | ||
| 274 | + (aset (:c f) ROWH 0.0)) | ||
| 275 | + cursor (ctr f CURSOR) | ||
| 253 | h (if grow-cross? rh mh) | 276 | h (if grow-cross? rh mh) |
| 254 | x (+ rx ox cursor) | 277 | x (+ rx ox cursor) |
| 255 | - y (+ ry oy (* gy (- rh h)))] | 278 | + y (+ ry oy (ctr f ROWY) (if (:wrap f) 0.0 (* gy (- rh h))))] |
| 256 | (tally! w mw mh grow-along?) | 279 | (tally! w mw mh grow-along?) |
| 257 | [x y w h])))) | 280 | [x y w h])))) |
| 258 | 281 | ||
| @@ -326,10 +349,14 @@ | |||
| 326 | "The container everything else is built from. | 349 | "The container everything else is built from. |
| 327 | 350 | ||
| 328 | opts: :dir :spacing :padding :margin :expand :gravity :key :fill :border | 351 | opts: :dir :spacing :padding :margin :expand :gravity :key :fill :border |
| 329 | - :radius :min-size :clip? :offset" | 352 | + :radius :min-size :clip? :offset :wrap |
| 353 | + | ||
| 354 | + :wrap only means anything on a row: children that will not fit start a | ||
| 355 | + new line instead of running off the end. A column already puts every | ||
| 356 | + child on a line of its own." | ||
| 330 | [opts body] | 357 | [opts body] |
| 331 | (let [{:keys [dir spacing padding margin expand gravity key fill border | 358 | (let [{:keys [dir spacing padding margin expand gravity key fill border |
| 332 | - radius min-size clip? offset fixed] | 359 | + radius min-size clip? offset fixed wrap] |
| 333 | :or {dir :vertical expand :none gravity [0.0 0.0] clip? false}} opts | 360 | :or {dir :vertical expand :none gravity [0.0 0.0] clip? false}} opts |
| 334 | spacing (double (or spacing 0.0)) | 361 | spacing (double (or spacing 0.0)) |
| 335 | padding (double (or padding 0.0)) | 362 | padding (double (or padding 0.0)) |
| @@ -371,6 +398,9 @@ | |||
| 371 | (when clip? (set-clip! (intersect (or outer-clip content) content))) | 398 | (when clip? (set-clip! (intersect (or outer-clip content) content))) |
| 372 | (push-frame! {:id id :dir dir :rect content :spacing spacing | 399 | (push-frame! {:id id :dir dir :rect content :spacing spacing |
| 373 | :share share :c (counters) | 400 | :share share :c (counters) |
| 401 | + ;; Only a row can wrap. A column already puts every child | ||
| 402 | + ;; on a line of its own. | ||
| 403 | + :wrap (boolean (and wrap (= dir :horizontal))) | ||
| 374 | :offset (or offset [0.0 0.0])}) | 404 | :offset (or offset [0.0 0.0])}) |
| 375 | (let [result (when body (body id box-rect)) | 405 | (let [result (when body (body id box-rect)) |
| 376 | f (pop-frame!)] | 406 | f (pop-frame!)] |
modified
jvui/src/jvui/widgets.clj +28 -5 | @@ -66,19 +66,42 @@ | ||
| 66 | 66 | |
| 67 | 67 | ;; -------------------------------------------------------------------- text |
| 68 | 68 | |
| 69 | +(defn- break-word | |
| 70 | + "Cut a word too long for `width` into pieces that fit. | |
| 71 | + | |
| 72 | + Character-wise and greedy. Only ever reached for a word that would not | |
| 73 | + fit on a line of its own — an ordinary sentence never comes here." | |
| 74 | + [w size width] | |
| 75 | + (loop [rest* w out []] | |
| 76 | + (if (or (empty? rest*) (<= (first (c/measure rest* size)) width)) | |
| 77 | + (if (seq rest*) (conj out rest*) out) | |
| 78 | + (let [n (loop [i 1] | |
| 79 | + (cond (>= i (count rest*)) (count rest*) | |
| 80 | + (> (first (c/measure (subs rest* 0 i) size)) width) (max 1 (dec i)) | |
| 81 | + :else (recur (inc i))))] | |
| 82 | + (recur (subs rest* n) (conj out (subs rest* 0 n))))))) | |
| 83 | + | |
| 69 | 84 | (defn- wrap-lines |
| 70 | 85 | "Break `s` into lines that each fit `width`, on spaces. |
| 71 | 86 | |
| 72 | - Greedy and word-wise, which is what a paragraph of UI text wants. A | |
| 73 | - single word longer than the line is left long rather than broken | |
| 74 | - mid-word: a URL cut in half is harder to read than one that overflows, | |
| 75 | - and the viewport clips it either way. | |
| 87 | + Greedy and word-wise, and a word that cannot fit on a line of its own | |
| 88 | + is broken at a character. That second half was left out first time | |
| 89 | + round on the grounds that a URL cut in half reads worse than one that | |
| 90 | + overflows — which was wrong, and the sign-in screen showed why: an | |
| 91 | + OAuth login URL is one unbreakable word, so it made its container | |
| 92 | + wider than the window and pushed the whole page off both edges. A | |
| 93 | + broken URL is worse than an unbroken one; a window you cannot read is | |
| 94 | + worse than both. | |
| 76 | 95 | |
| 77 | 96 | Measured through the same `c/measure` the drawing uses, so a line that |
| 78 | 97 | is said to fit does fit — a wrap computed against a different metric |
| 79 | 98 | than the renderer's is off by a word at the worst moments." |
| 80 | 99 | [s size width] |
| 81 | - (let [words (str/split (str s) #" ")] | |
| 100 | + (let [words (mapcat (fn [w] | |
| 101 | + (if (> (first (c/measure w size)) width) | |
| 102 | + (break-word w size width) | |
| 103 | + [w])) | |
| 104 | + (str/split (str s) #" "))] | |
| 82 | 105 | (loop [[w & more] words line nil out []] |
| 83 | 106 | (cond |
| 84 | 107 | (nil? w) (if line (conj out line) out) |
| @@ -66,19 +66,42 @@ | |||
| 66 | 66 | ||
| 67 | ;; -------------------------------------------------------------------- text | 67 | ;; -------------------------------------------------------------------- text |
| 68 | 68 | ||
| 69 | +(defn- break-word | ||
| 70 | + "Cut a word too long for `width` into pieces that fit. | ||
| 71 | + | ||
| 72 | + Character-wise and greedy. Only ever reached for a word that would not | ||
| 73 | + fit on a line of its own — an ordinary sentence never comes here." | ||
| 74 | + [w size width] | ||
| 75 | + (loop [rest* w out []] | ||
| 76 | + (if (or (empty? rest*) (<= (first (c/measure rest* size)) width)) | ||
| 77 | + (if (seq rest*) (conj out rest*) out) | ||
| 78 | + (let [n (loop [i 1] | ||
| 79 | + (cond (>= i (count rest*)) (count rest*) | ||
| 80 | + (> (first (c/measure (subs rest* 0 i) size)) width) (max 1 (dec i)) | ||
| 81 | + :else (recur (inc i))))] | ||
| 82 | + (recur (subs rest* n) (conj out (subs rest* 0 n))))))) | ||
| 83 | + | ||
| 69 | (defn- wrap-lines | 84 | (defn- wrap-lines |
| 70 | "Break `s` into lines that each fit `width`, on spaces. | 85 | "Break `s` into lines that each fit `width`, on spaces. |
| 71 | 86 | ||
| 72 | - Greedy and word-wise, which is what a paragraph of UI text wants. A | 87 | + Greedy and word-wise, and a word that cannot fit on a line of its own |
| 73 | - single word longer than the line is left long rather than broken | 88 | + is broken at a character. That second half was left out first time |
| 74 | - mid-word: a URL cut in half is harder to read than one that overflows, | 89 | + round on the grounds that a URL cut in half reads worse than one that |
| 75 | - and the viewport clips it either way. | 90 | + overflows — which was wrong, and the sign-in screen showed why: an |
| 91 | + OAuth login URL is one unbreakable word, so it made its container | ||
| 92 | + wider than the window and pushed the whole page off both edges. A | ||
| 93 | + broken URL is worse than an unbroken one; a window you cannot read is | ||
| 94 | + worse than both. | ||
| 76 | 95 | ||
| 77 | Measured through the same `c/measure` the drawing uses, so a line that | 96 | Measured through the same `c/measure` the drawing uses, so a line that |
| 78 | is said to fit does fit — a wrap computed against a different metric | 97 | is said to fit does fit — a wrap computed against a different metric |
| 79 | than the renderer's is off by a word at the worst moments." | 98 | than the renderer's is off by a word at the worst moments." |
| 80 | [s size width] | 99 | [s size width] |
| 81 | - (let [words (str/split (str s) #" ")] | 100 | + (let [words (mapcat (fn [w] |
| 101 | + (if (> (first (c/measure w size)) width) | ||
| 102 | + (break-word w size width) | ||
| 103 | + [w])) | ||
| 104 | + (str/split (str s) #" "))] | ||
| 82 | (loop [[w & more] words line nil out []] | 105 | (loop [[w & more] words line nil out []] |
| 83 | (cond | 106 | (cond |
| 84 | (nil? w) (if line (conj out line) out) | 107 | (nil? w) (if line (conj out line) out) |
modified
jvui/test/jvui/tests.clj +36 -0 | @@ -268,6 +268,40 @@ | ||
| 268 | 268 | (check! (= [127 127 127 255] (theme/mix [0 0 0 255] [254 254 254 255] 0.5)) |
| 269 | 269 | "and halfway")) |
| 270 | 270 | |
| 271 | +(defn- check-long-word-breaks! [] | |
| 272 | + ;; An OAuth login URL is ONE word with no spaces in it. Word-wise | |
| 273 | + ;; wrapping leaves it long, it makes its container wider than the | |
| 274 | + ;; window, and the sign-in page then hangs off both edges — which is | |
| 275 | + ;; exactly what frq did. A broken URL is worse than an unbroken one; a | |
| 276 | + ;; window you cannot read is worse than both. | |
| 277 | + (let [url (str "https://freeq.at/auth/login?handle=nandi-test.bsky.social" | |
| 278 | + "&return_to=http%3A%2F%2F127.0.0.1%3A7390%2Fcallback") | |
| 279 | + cx (ctx) seen (atom nil)] | |
| 280 | + (dotimes [_ 2] | |
| 281 | + (frame! cx (fn [] (c/box* {:dir :vertical} | |
| 282 | + (fn [_ _] (reset! seen (w/label url))))))) | |
| 283 | + (let [[_ _ w h] @seen] | |
| 284 | + (check! (<= w 400.5) "an unbreakable word is broken to fit") | |
| 285 | + (check! (> h 16.5) "over as many lines as it takes")))) | |
| 286 | + | |
| 287 | +(defn- check-row-wraps! [] | |
| 288 | + ;; A row of reaction pills has to start a new line rather than run off | |
| 289 | + ;; the end of the message it belongs to. | |
| 290 | + (let [cx (ctx) seen (atom [])] | |
| 291 | + (dotimes [_ 2] | |
| 292 | + (reset! seen []) | |
| 293 | + (frame! cx (fn [] (c/box* {:dir :horizontal :wrap true :spacing 4} | |
| 294 | + (fn [_ _] | |
| 295 | + (dotimes [i 12] | |
| 296 | + (swap! seen conj | |
| 297 | + (w/label (str "pill" i) {:expand :none})))))))) | |
| 298 | + (let [ys (distinct (map second @seen)) | |
| 299 | + xs (map first @seen)] | |
| 300 | + (check! (> (count ys) 1) | |
| 301 | + (str "a wrapped row used more than one line: " (count ys))) | |
| 302 | + (check! (<= (apply max xs) 400.0) | |
| 303 | + (str "and none of it ran off the end: max x=" (apply max xs)))))) | |
| 304 | + | |
| 271 | 305 | (defn- check-label-wraps! [] |
| 272 | 306 | ;; A sentence longer than its box must come back TALLER and no wider, not |
| 273 | 307 | ;; wider and on one line. Without this a single long label makes its |
| @@ -296,6 +330,8 @@ | ||
| 296 | 330 | ["a column stacks" check-vbox-stacks!] |
| 297 | 331 | ["a row runs" check-hbox-runs!] |
| 298 | 332 | ["a long label wraps" check-label-wraps!] |
| 333 | + ["an unbreakable word breaks" check-long-word-breaks!] | |
| 334 | + ["a row wraps" check-row-wraps!] | |
| 299 | 335 | ["expand shares the slack" check-expand-shares!] |
| 300 | 336 | ["gravity centres" check-gravity-centres!] |
| 301 | 337 | ["layout settles unseen" check-layout-settles!] |
| @@ -268,6 +268,40 @@ | |||
| 268 | (check! (= [127 127 127 255] (theme/mix [0 0 0 255] [254 254 254 255] 0.5)) | 268 | (check! (= [127 127 127 255] (theme/mix [0 0 0 255] [254 254 254 255] 0.5)) |
| 269 | "and halfway")) | 269 | "and halfway")) |
| 270 | 270 | ||
| 271 | +(defn- check-long-word-breaks! [] | ||
| 272 | + ;; An OAuth login URL is ONE word with no spaces in it. Word-wise | ||
| 273 | + ;; wrapping leaves it long, it makes its container wider than the | ||
| 274 | + ;; window, and the sign-in page then hangs off both edges — which is | ||
| 275 | + ;; exactly what frq did. A broken URL is worse than an unbroken one; a | ||
| 276 | + ;; window you cannot read is worse than both. | ||
| 277 | + (let [url (str "https://freeq.at/auth/login?handle=nandi-test.bsky.social" | ||
| 278 | + "&return_to=http%3A%2F%2F127.0.0.1%3A7390%2Fcallback") | ||
| 279 | + cx (ctx) seen (atom nil)] | ||
| 280 | + (dotimes [_ 2] | ||
| 281 | + (frame! cx (fn [] (c/box* {:dir :vertical} | ||
| 282 | + (fn [_ _] (reset! seen (w/label url))))))) | ||
| 283 | + (let [[_ _ w h] @seen] | ||
| 284 | + (check! (<= w 400.5) "an unbreakable word is broken to fit") | ||
| 285 | + (check! (> h 16.5) "over as many lines as it takes")))) | ||
| 286 | + | ||
| 287 | +(defn- check-row-wraps! [] | ||
| 288 | + ;; A row of reaction pills has to start a new line rather than run off | ||
| 289 | + ;; the end of the message it belongs to. | ||
| 290 | + (let [cx (ctx) seen (atom [])] | ||
| 291 | + (dotimes [_ 2] | ||
| 292 | + (reset! seen []) | ||
| 293 | + (frame! cx (fn [] (c/box* {:dir :horizontal :wrap true :spacing 4} | ||
| 294 | + (fn [_ _] | ||
| 295 | + (dotimes [i 12] | ||
| 296 | + (swap! seen conj | ||
| 297 | + (w/label (str "pill" i) {:expand :none})))))))) | ||
| 298 | + (let [ys (distinct (map second @seen)) | ||
| 299 | + xs (map first @seen)] | ||
| 300 | + (check! (> (count ys) 1) | ||
| 301 | + (str "a wrapped row used more than one line: " (count ys))) | ||
| 302 | + (check! (<= (apply max xs) 400.0) | ||
| 303 | + (str "and none of it ran off the end: max x=" (apply max xs)))))) | ||
| 304 | + | ||
| 271 | (defn- check-label-wraps! [] | 305 | (defn- check-label-wraps! [] |
| 272 | ;; A sentence longer than its box must come back TALLER and no wider, not | 306 | ;; A sentence longer than its box must come back TALLER and no wider, not |
| 273 | ;; wider and on one line. Without this a single long label makes its | 307 | ;; wider and on one line. Without this a single long label makes its |
| @@ -296,6 +330,8 @@ | |||
| 296 | ["a column stacks" check-vbox-stacks!] | 330 | ["a column stacks" check-vbox-stacks!] |
| 297 | ["a row runs" check-hbox-runs!] | 331 | ["a row runs" check-hbox-runs!] |
| 298 | ["a long label wraps" check-label-wraps!] | 332 | ["a long label wraps" check-label-wraps!] |
| 333 | + ["an unbreakable word breaks" check-long-word-breaks!] | ||
| 334 | + ["a row wraps" check-row-wraps!] | ||
| 299 | ["expand shares the slack" check-expand-shares!] | 335 | ["expand shares the slack" check-expand-shares!] |
| 300 | ["gravity centres" check-gravity-centres!] | 336 | ["gravity centres" check-gravity-centres!] |
| 301 | ["layout settles unseen" check-layout-settles!] | 337 | ["layout settles unseen" check-layout-settles!] |