Give every entry a key, and fail the build for one without
An entry with no `:key` falls back to a single shared TextEditingController on the phone, so two unkeyed entries on a screen are one controller and whichever renders last wins. The emoji search and the composer are both on the chat screen, the composer renders after it holding an empty draft, and so the search was wiped after every keystroke — one character, every time. This is the second time. The connect screen's host field showed the port for the same reason and was fixed by keying those six entries, which left the other four exactly as they were and the trap exactly where it was. So `tools/check-common.py` now fails on an `[:entry ...]` written without one, which is the only thing that would have stopped it coming back. glimmer wants the key too, to match children across a render, so this is what the desktop wanted anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0ee5786 parent: fc5f63c modified
common/frq/screens/chat.cljc +4 -2 | @@ -356,7 +356,8 @@ | ||
| 356 | 356 | searching? (seq (str/trim @cells/emoji-search))] |
| 357 | 357 | [:vbox {:key :picker :spacing 4} |
| 358 | 358 | [:hbox {:spacing 6} |
| 359 | - [:entry {:text @cells/emoji-search | |
| 359 | + [:entry {:key :emoji-search | |
| 360 | + :text @cells/emoji-search | |
| 360 | 361 | :width-request 240 |
| 361 | 362 | :placeholder "Search emoji" |
| 362 | 363 | :on-change #(reset! cells/emoji-search %)}] |
| @@ -1370,7 +1371,8 @@ | ||
| 1370 | 1371 | ;; being typed in: rows spent on a paragraph nobody had written, taken |
| 1371 | 1372 | ;; off the conversation above. Now the box is a line until there is a |
| 1372 | 1373 | ;; second line to put in it. |
| 1373 | - [:entry {:text @cells/draft | |
| 1374 | + [:entry {:key :draft | |
| 1375 | + :text @cells/draft | |
| 1374 | 1376 | :width-request 260 |
| 1375 | 1377 | ;; Always, not only in a terminal. A window is the case that |
| 1376 | 1378 | ;; needs it more: 260 points is most of a phone's row and a |
| @@ -356,7 +356,8 @@ | |||
| 356 | searching? (seq (str/trim @cells/emoji-search))] | 356 | searching? (seq (str/trim @cells/emoji-search))] |
| 357 | [:vbox {:key :picker :spacing 4} | 357 | [:vbox {:key :picker :spacing 4} |
| 358 | [:hbox {:spacing 6} | 358 | [:hbox {:spacing 6} |
| 359 | - [:entry {:text @cells/emoji-search | 359 | + [:entry {:key :emoji-search |
| 360 | + :text @cells/emoji-search | ||
| 360 | :width-request 240 | 361 | :width-request 240 |
| 361 | :placeholder "Search emoji" | 362 | :placeholder "Search emoji" |
| 362 | :on-change #(reset! cells/emoji-search %)}] | 363 | :on-change #(reset! cells/emoji-search %)}] |
| @@ -1370,7 +1371,8 @@ | |||
| 1370 | ;; being typed in: rows spent on a paragraph nobody had written, taken | 1371 | ;; being typed in: rows spent on a paragraph nobody had written, taken |
| 1371 | ;; off the conversation above. Now the box is a line until there is a | 1372 | ;; off the conversation above. Now the box is a line until there is a |
| 1372 | ;; second line to put in it. | 1373 | ;; second line to put in it. |
| 1373 | - [:entry {:text @cells/draft | 1374 | + [:entry {:key :draft |
| 1375 | + :text @cells/draft | ||
| 1374 | :width-request 260 | 1376 | :width-request 260 |
| 1375 | ;; Always, not only in a terminal. A window is the case that | 1377 | ;; Always, not only in a terminal. A window is the case that |
| 1376 | ;; needs it more: 260 points is most of a phone's row and a | 1378 | ;; needs it more: 260 points is most of a phone's row and a |
modified
common/frq/screens/chats.cljc +4 -2 | @@ -185,7 +185,8 @@ | ||
| 185 | 185 | :on-click #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}] |
| 186 | 186 | ;; Both kinds of conversation through one box: `#room` joins a |
| 187 | 187 | ;; channel, `@nick` opens a message to a person. |
| 188 | - [:entry {:text @cells/join-input | |
| 188 | + [:entry {:key :join-input | |
| 189 | + :text @cells/join-input | |
| 189 | 190 | :placeholder "#channel or @nick" |
| 190 | 191 | :on-change #(reset! cells/join-input %) |
| 191 | 192 | :on-activate #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}]] |
| @@ -196,7 +197,8 @@ | ||
| 196 | 197 | [:hbox {:spacing 8 :align :end} |
| 197 | 198 | (when (seq @cells/search) |
| 198 | 199 | [:button {:label "✕" :on-click #(reset! cells/search "")}]) |
| 199 | - [:entry {:text @cells/search | |
| 200 | + [:entry {:key :search | |
| 201 | + :text @cells/search | |
| 200 | 202 | :placeholder "Search channels" |
| 201 | 203 | :on-change #(reset! cells/search %)}]]]]] |
| 202 | 204 | ;; `:fill-height` so the scroll inside is handed the rest of the window |
| @@ -185,7 +185,8 @@ | |||
| 185 | :on-click #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}] | 185 | :on-click #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}] |
| 186 | ;; Both kinds of conversation through one box: `#room` joins a | 186 | ;; Both kinds of conversation through one box: `#room` joins a |
| 187 | ;; channel, `@nick` opens a message to a person. | 187 | ;; channel, `@nick` opens a message to a person. |
| 188 | - [:entry {:text @cells/join-input | 188 | + [:entry {:key :join-input |
| 189 | + :text @cells/join-input | ||
| 189 | :placeholder "#channel or @nick" | 190 | :placeholder "#channel or @nick" |
| 190 | :on-change #(reset! cells/join-input %) | 191 | :on-change #(reset! cells/join-input %) |
| 191 | :on-activate #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}]] | 192 | :on-activate #(do (actions/join! @cells/join-input) (reset! cells/join-input ""))}]] |
| @@ -196,7 +197,8 @@ | |||
| 196 | [:hbox {:spacing 8 :align :end} | 197 | [:hbox {:spacing 8 :align :end} |
| 197 | (when (seq @cells/search) | 198 | (when (seq @cells/search) |
| 198 | [:button {:label "✕" :on-click #(reset! cells/search "")}]) | 199 | [:button {:label "✕" :on-click #(reset! cells/search "")}]) |
| 199 | - [:entry {:text @cells/search | 200 | + [:entry {:key :search |
| 201 | + :text @cells/search | ||
| 200 | :placeholder "Search channels" | 202 | :placeholder "Search channels" |
| 201 | :on-change #(reset! cells/search %)}]]]]] | 203 | :on-change #(reset! cells/search %)}]]]]] |
| 202 | ;; `:fill-height` so the scroll inside is handed the rest of the window | 204 | ;; `:fill-height` so the scroll inside is handed the rest of the window |
modified
tools/check-common.py +29 -0 | @@ -205,6 +205,34 @@ def cells_enumerated(root): | ||
| 205 | 205 | ] |
| 206 | 206 | |
| 207 | 207 | |
| 208 | +def entries_keyed(root): | |
| 209 | + """Every `[:entry ...]` in a shared screen carries a `:key`. | |
| 210 | + | |
| 211 | + The phone keeps one TextEditingController per key, and an entry without | |
| 212 | + one falls back to a single shared controller — so two unkeyed entries on a | |
| 213 | + screen are the same controller, and whichever renders last wins. It has | |
| 214 | + cost two bugs: the connect screen's host field showed the port, and the | |
| 215 | + emoji search would not hold more than one character, because the composer | |
| 216 | + rendered after it with an empty draft and wiped it. | |
| 217 | + | |
| 218 | + glimmer wants the key too, to match children across a render. Nothing | |
| 219 | + enforced it, which is why it kept coming back. | |
| 220 | + """ | |
| 221 | + bad = [] | |
| 222 | + for path in sorted(root.rglob("*.cljc")): | |
| 223 | + src = strip(path.read_text()) | |
| 224 | + for i, line in enumerate(src.split("\n")): | |
| 225 | + if "[:entry" not in line: | |
| 226 | + continue | |
| 227 | + # the props map may run over a few lines; :key belongs in it | |
| 228 | + blob = "\n".join(src.split("\n")[i:i + 8]) | |
| 229 | + if ":key" not in blob: | |
| 230 | + bad.append((path, i + 1, ":entry", | |
| 231 | + "written without a :key — unkeyed entries share one " | |
| 232 | + "text controller on the phone")) | |
| 233 | + return bad | |
| 234 | + | |
| 235 | + | |
| 208 | 236 | def main(): |
| 209 | 237 | root = Path(sys.argv[1] if len(sys.argv) > 1 else "common") |
| 210 | 238 | bad = [] |
| @@ -216,6 +244,7 @@ def main(): | ||
| 216 | 244 | if m: |
| 217 | 245 | bad.append((path, lineno, m.group(0).strip(), why)) |
| 218 | 246 | bad += cells_enumerated(root) |
| 247 | + bad += entries_keyed(root) | |
| 219 | 248 | for path, lineno, tok, why in bad: |
| 220 | 249 | where = f"{path}:{lineno}" if lineno else str(path) |
| 221 | 250 | print(f"{where}: {tok!r} is {why}", file=sys.stderr) |
| @@ -205,6 +205,34 @@ def cells_enumerated(root): | |||
| 205 | ] | 205 | ] |
| 206 | 206 | ||
| 207 | 207 | ||
| 208 | +def entries_keyed(root): | ||
| 209 | + """Every `[:entry ...]` in a shared screen carries a `:key`. | ||
| 210 | + | ||
| 211 | + The phone keeps one TextEditingController per key, and an entry without | ||
| 212 | + one falls back to a single shared controller — so two unkeyed entries on a | ||
| 213 | + screen are the same controller, and whichever renders last wins. It has | ||
| 214 | + cost two bugs: the connect screen's host field showed the port, and the | ||
| 215 | + emoji search would not hold more than one character, because the composer | ||
| 216 | + rendered after it with an empty draft and wiped it. | ||
| 217 | + | ||
| 218 | + glimmer wants the key too, to match children across a render. Nothing | ||
| 219 | + enforced it, which is why it kept coming back. | ||
| 220 | + """ | ||
| 221 | + bad = [] | ||
| 222 | + for path in sorted(root.rglob("*.cljc")): | ||
| 223 | + src = strip(path.read_text()) | ||
| 224 | + for i, line in enumerate(src.split("\n")): | ||
| 225 | + if "[:entry" not in line: | ||
| 226 | + continue | ||
| 227 | + # the props map may run over a few lines; :key belongs in it | ||
| 228 | + blob = "\n".join(src.split("\n")[i:i + 8]) | ||
| 229 | + if ":key" not in blob: | ||
| 230 | + bad.append((path, i + 1, ":entry", | ||
| 231 | + "written without a :key — unkeyed entries share one " | ||
| 232 | + "text controller on the phone")) | ||
| 233 | + return bad | ||
| 234 | + | ||
| 235 | + | ||
| 208 | def main(): | 236 | def main(): |
| 209 | root = Path(sys.argv[1] if len(sys.argv) > 1 else "common") | 237 | root = Path(sys.argv[1] if len(sys.argv) > 1 else "common") |
| 210 | bad = [] | 238 | bad = [] |
| @@ -216,6 +244,7 @@ def main(): | |||
| 216 | if m: | 244 | if m: |
| 217 | bad.append((path, lineno, m.group(0).strip(), why)) | 245 | bad.append((path, lineno, m.group(0).strip(), why)) |
| 218 | bad += cells_enumerated(root) | 246 | bad += cells_enumerated(root) |
| 247 | + bad += entries_keyed(root) | ||
| 219 | for path, lineno, tok, why in bad: | 248 | for path, lineno, tok, why in bad: |
| 220 | where = f"{path}:{lineno}" if lineno else str(path) | 249 | where = f"{path}:{lineno}" if lineno else str(path) |
| 221 | print(f"{where}: {tok!r} is {why}", file=sys.stderr) | 250 | print(f"{where}: {tok!r} is {why}", file=sys.stderr) |