feat(packets): add filter to All Observer dropdown - #1884
Conversation
Lets users type a prefix to filter the observer checkbox list, instead of scrolling a long list.
|
Review from the queue triage. Written 2026-08-30 against the tree at that time; posting now that the maintenance window in #1922 has opened. Verdict: request changes. Two findings, one user-visible.
No cache-buster comment needed: this repo bumps automatically, see repo facts above. |
|
Recycling this again, and the reason is my mistake rather than anything about your PR. Earlier today I approved the pending workflow run on this PR. That was the wrong order: approving an Closing and reopening now gets a fresh merge commit against current master, which is what the run should have been all along. No action needed from you, and apologies for the second round of noise. |
|
Recycling for a fresh run. The failure looks unrelated to your change, but it is not the flake that was hitting everything else, so it is worth watching. Your pipeline failed on: That is a keyboard-focus test on the customizer colour swatches, which belongs to the focus-race family this repo already has history with (#1172, #1616). It is not #1923 (the packets time-window bug) and not #1925 (the neighbour-graph race). Your PR adds a search input to the observer dropdown in If it fails the same way twice, it is worth a proper look rather than another re-run. My review above still stands on the |
|
Follow-up on my note above: I said that if it failed the same way twice it would be worth a proper look. It passed on the rerun, so the The review point above is still open and is the only thing holding this: The second point is smaller: |
…diately (#1939) This is the test that has been keeping master red on both sides of today's queue run. ## The failure ``` ✗ ArrowRight cycles focus across swatches: ArrowRight should move focus to next swatch (was #ef4444, now #ef4444) ``` Observed on: | where | when | |---|---| | master push `589fa987` | 2026-08-31 — the last completed master run before today | | master push `859173f1` | 2026-09-02 — the first completed master run after #1938 | | PR #1884 | 2026-09-02, passed unchanged on a re-run | **Two out of two completed master runs.** Master has produced exactly two finished pipelines since 2026-08-31 and this test failed both, which is why the branch has had no green badge either side of a day of merges. ## The cause ```js await page.keyboard.press('ArrowRight'); const nextColor = await page.evaluate(() => document.activeElement.getAttribute('data-color')); ``` It reads `document.activeElement` on the tick after the key press. The keydown handler moves focus, but under CI load that can land after the evaluate has already run, so the assertion compares the swatch against itself and reports the same colour twice. **This file already knows about this.** The "outside click" step below carries a long comment about exactly this macrotask race for #1317, and the conclusion there was to wait on the real condition instead of a proxy. That step got the treatment and this one did not. ## The fix Wait for `activeElement` to be a `.cc-swatch` whose `data-color` differs from the one focused before the key press, with a 3s budget. The wait is wrapped so that a timeout falls through to the original assertion, which then reports the value actually observed rather than a bare Playwright timeout — a failing test should still say what it saw. **No product code changed.** One file, +18 lines, all of it the wait and the reasoning. ## What this does not claim It does not prove the focus handler is correct, only that the test stops racing it. If ArrowRight is ever genuinely broken, this still fails, and now with a useful message. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Wzwr3eXseyNM7Xj598djjE --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Lets users type a prefix to filter the observer checkbox list, instead of scrolling a long list.