feat(app,web): make the two filter panels one panel - #573
Open
khagele wants to merge 15 commits into
Open
Conversation
The card was ten lanes or nothing: 298px, a third of a 915px phone, whether
it held one reception or two hundred. A full card did not even show ten. The
playhead sat six lanes down with three lanes of padding under it, and the
fade reached zero on the card's own top lane, so ten receptions rendered as
six readable rows, one active row, and three blank lanes.
The height now follows the content, in the steps Kasper specified:
0 -> header only 3..5 -> 3 lanes
1..2 -> 1 lane 6..9 -> 5 lanes
10+ -> 10 lanes
Below six receptions the card shows everything it has; from six it caps, so
the oldest roll off the top rather than the card taking the screen.
Two things follow from "newest at the bottom, at every size":
- The playhead is the bottom lane now, not six lanes down, and the padding
under it is zero. That is what makes ten lanes mean ten receptions.
- The fade had to follow. Its old divisor of 6 was the old geometry written
twice, and it reached zero exactly on the top lane, so the height promised
rows the opacity then hid. It now spans the lanes there actually are above
the playhead and stops at a floor. Measured on an eleven-reception card:
ten rows visible, opacity 0.22 at the top through to 1 on the newest.
The chevron beside the close button has three stops rather than two: full,
three lanes, one lane, then back to full. It points down while there is
further to collapse and up on the last stop, so it stays one control that
swaps state. It appears only once the card is taller than its smallest stop,
which is from three receptions; below that it would do nothing.
The geometry is four numbers that have to agree, so they are derived from one
another rather than kept in step by hand. rxLanes, rxPlayhead and rxPadBottom
are pure and unit-tested, and the stylesheet multiplies the lane counts they
publish by --ch-rx-line-h. playhead + padBottom is always lanes - 1, which is
exactly the condition under which the scroll code can still reach the newest
reception; that is asserted, not assumed. The curve is pinned as the table
above rather than as the thresholds the code uses, so a wrong implementation
disagrees with it instead of restating it.
Every on-screen state is one stored value under core-hunter-ticker: open,
collapsed, minimal, closed. A boolean plus a size would let a reload land on
"closed and expanded", which is not a state, and the round trip is asserted
for every stop the chevron can reach. 'open' and 'closed' are what pre-efiten#560
builds wrote, so existing installs keep working and anything unrecognised
reads as a first visit.
Closing travels towards the top-bar button. The card used to vanish on the
spot, leaving nothing connecting it to the control that now held it.
web/parity.test.js's efiten#322 guard pinned .rx-list's height to the literal
10-lane calc on both surfaces. Its subject is the pitch, not the number, so
it now asserts that neither surface bakes a pixel value, then pins the map's
literals and the app's sources separately. It also pins the divergence this
creates: the map still puts the playhead six lanes down with three below,
the app puts the newest on the bottom lane. efiten#424 is where the map follows.
Measured in the browser at 412x915 against the specified table: 0 gives the
header only, 1 and 2 give 1 lane, 3 to 5 give 3, 6 and 9 give 5, 10 and 11
give 10, with zero pixels under the newest reception at every step and the
newest always the last visible row. The chevron cycle was measured too:
10 lanes, 3, 1, back to 10, storing open, collapsed, minimal, open.
A first attempt animated the height, which fires scroll events mid-flight;
the scroll handler reads atBottom() against a box that is still moving, so
`follow` latched off and the card stopped following the newest line. The
transition is gone.
Not verified here: the closing travel itself. This preview pane runs no CSS
transitions at all, confirmed with a control probe on a throwaway element
whose opacity read 1 at 100ms of a 200ms transition, and document.hidden is
true. The end state is verified: the card hides, the button appears, the
state stores, and no class or custom property is left behind.
Tests: app 928 passed + build clean, web 509 passed, eslint clean. The
geometry assertions were mutation-checked, and the rewritten parity guard was
checked by baking a pixel height back into the app's rule.
Note for a separate issue: soundengine.test.js's "drifts them" case fails
intermittently on upstream master too (1 of 3 full runs at 90c3375, same
message), so CI can redden on it at random. Untouched here.
Closes efiten#560
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…it instead The first cut of efiten#560 moved the playhead to the bottom lane to satisfy "newest at the bottom". That worked, and it threw away what efiten#130's playhead is for: lines rolling through a fixed lane with newer receptions below it. Once the playhead was the bottom lane there was no below, so scrubbing back showed only older receptions and the newer ones fell off the edge. The two were never in conflict. The blank lanes a full card ended in came from the padding under the last row, not from where the playhead sat. With that padding at zero the browser clamps the follow-scroll three lanes short of the playhead, and that clamp parks the newest reception on the bottom lane while the playhead stays six lanes down. Measured at 412x915: playhead on lane 6, three receptions below it, newest on the bottom lane, zero pixels under it, ten rows on a ten-lane card. - rxPlayhead keeps the 6-of-9 proportion at every size, so a five-lane card puts it on lane 3 and a three-lane card on lane 1. Never above the middle, which is what keeps the older context the larger half. - rxBelow is new: how many lanes sit under the playhead. - rxPadBottom is zero and stays a function, because the reachability assertion is written against it. - maxScroll now reads the browser's own maximum rather than (rows - 1) * lineH. With no padding under the last row the list cannot scroll far enough to put that row on the playhead, so comparing against the JS lane count made atBottom() permanently false, which latches `follow` off and stops the card following live traffic. - rxFade takes both spans. Its fixed divisors reached zero on the outermost lane of each side, which was harmless while those lanes were blank padding and is not now that they hold receptions: the newest row on a full card sits exactly three below the playhead. Each side now fades to the floor instead. The parity guard's divergence note was wrong on this point and is corrected: both surfaces keep the playhead six lanes down, and what differs is the padding under the last row, which the map still reserves. Tests: app 935 passed + build clean, web 509 passed, eslint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…k to The list's padding-bottom fell back to three lanes, which is what the card reserved before this branch. rxPadBottom returns zero at every size now, so a card rendered before its first rebuild would show the blank lanes the change is about, and then lose them a tick later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two tickers had made opposite decisions on almost every point. The app's sits on a plate, closes, remembers its size and follows its content; the map's lay straight on the tiles with two states, full or the header alone. F3 of the 26 August review was "the ticker has no backing plate, so it is unreadable over the map". It was agreed and then fixed only in the app. ## What the map gets - **A plate.** `--ch-surface-thin` plus a blur, the app's. `pointer-events` stays `none` on the box, so the plate paints and drags and wheels still reach Leaflet, which is the bargain efiten#287/efiten#322 set up. - **The height follows the content**, in efiten#560's steps: nothing but the header, one lane, three, five, ten. The geometry functions are mirrored from the app and parity.test.js now compares them by running both copies, over every count from 0 to 60, rather than by reading their constants. - **The newest reception on the bottom lane**, with the playhead still six lanes down and receptions rolling through below it. That came from dropping the padding under the last row, which is what reserved the blank lanes. - **Four stops on one control**, and the chevron says which way it goes. ## What it deliberately keeps Dragging, and its top-right anchor. Folding to the header stays the way the ticker is put away here, and is simply the last stop rather than a second control: the map has no top-bar button to travel to, and adding one now would be built twice, since efiten#561 is a design round over that whole bar. That is the only difference the parity guard still pins, and it pins the shared stops as equal. ## Three things measured rather than read - **The plate resolved to `rgba(0,0,0,0)`.** `--ch-surface-thin` was declared in the app's tokens by efiten#539 and nowhere on the map, so the card was a border and a blur with nothing behind them. The token is now declared for both themes with the app's values, and the guard pins that they match. - **A stop of zero lanes is falsy.** `cap ? Math.min(lanes, cap) : lanes` silently ignored the header-alone stop, so its click did nothing at all. It tests `=== undefined` now. Found by clicking through it; the unit tests did not cover a zero stop, and now do, mutation-checked. - **The stored level cannot be a bare number.** Links from before this carry 0 or 1 for expanded or folded, and folded meant the header alone, which is now the last of four. Writing the new level 1 as "1" would make an old link and a new one disagree, so the stops above full serialise as letters. Round trip asserted for every level, and `1` still reads as the header. The placement block runs before `createReceptionTicker` does, so the level restored from the URL could not reach the component from inside it; it is applied once the ticker exists. ## Tests The e2e walked one click to hidden. It now walks the cycle instead, asserting every click shrinks the card until the list is hidden and one more returns to full, without assuming how many stops there are: that depends on the traffic, since a stop that would not make the card smaller is skipped. Its fixture went from one reception to twelve, which is past the last step. - web: 517 unit, 199 e2e with --fail-on-flaky-tests - app: 935 unit, build clean - eslint: clean Verified in the browser at 1280x760 with fourteen receptions, dark and light: ten lanes, ten rows shown, zero pixels under the newest, faintest row on the floor, and the plate resolving in both themes. Closes efiten#424 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The map's ticker got efiten#560's behaviour but not its chrome: a header with no height and no rule under it, rows flush to the plate's edge, and a text glyph where the app draws a button. Nothing in the tests could see that, so it took someone looking at the two side by side. That is the pattern worth fixing, not the ticker. `docs/design-system.md` is the register: one system across the three surfaces, every control named as the standard SaaS pattern it is an instance of, and each deliberate difference written down as a rule about the SURFACE rather than an exception for one component. AGENTS.md §7 points at it. The surface rule this one produced: a floating panel over the map is draggable, closable and collapsible, and passes pointer events through to Leaflet. That now covers whatever is added to web/ next, not just the ticker. The map's header, list, row and column rules are the app's now. parity.test.js compares those six rules declaration by declaration and pins the fold button's box against the app's, so the chrome cannot drift again without a test saying so. Mutation-checked by putting the old header back. If a proposal cannot be named as a mainstream pattern, it is bespoke. That naming is in the register on purpose: it is the test that catches an invented control before it ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g it back Putting the ticker away was the one thing still done differently on each surface: the app has a cross on the card and a button in the top bar, the map folded to its own header because it had no button to come back from. Kasper's call: give the map the same two, rather than keep the difference. So the map's bar carries a #ticker-btn beside the menu, in the same place as the app's, and the header carries the same cross beside the same chevron. The collapse stops are now the app's exactly, three lanes and one: away is the cross, not a further stop, and the zero-lane stop is gone. The stored field carries all four states, the way the app's one key does. A size plus a separate visible flag would let a link land on "away and expanded", which is not a state. Links from before this carry 0 or 1, and 1 was how the ticker was put away, so it reads as away rather than as a shrink stop; the states are written as letters so nothing new can be mistaken for it. A phone now starts at the smallest stop rather than away. The reason that default is per-surface is only that the card should not cover the map there, and a ticker nobody can see is a different thing from a one-line one. ## The register `docs/design-system.md` takes the three other decisions from the same round: - **The app uses sheets, the map uses floating panels**, as a named surface rule with the standard pattern behind each. What is inside them does not differ. - **44px is the rule except where the target lies over the map**, which turns the node markers from an exception into the carve-out every map product makes: the target swallows the pan gesture that starts on it. - **Closing works the same on both surfaces**, a cross plus a button in the bar. A panel that can be dismissed with no visible way back is not dismissible, it is lost. Notices and empty states, forms, and onboarding are named as the next sections rather than written from memory: each needs the code read first, and where the surfaces already disagree that is a finding, not something to smooth over. ## Tests The e2e now walks the shrink cycle without assuming its length, asserts it never reaches zero, and covers the cross and the bar button including across a reload. Two guards I had broken are repaired: the empty-state rule targets the list by id again, because parity.test.js's older matcher takes the first rule mentioning the class, and the stops guard asserts the two surfaces are equal rather than that the map has one more. - web: 518 unit, 200 e2e with --fail-on-flaky-tests - app: 935 unit, build clean - eslint: clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… menu The receptions button and the menu sit next to each other in both bars, and both drew three horizontal lines: one set staggered, one set equal. At 20px, side by side, that reads as two hamburgers. Kasper caught it on the map's bar, where efiten#424 had just put the two together; the app has had the same pair since efiten#539. It is a pulse now, which is what a live feed is drawn as. Changed on both surfaces in one commit, because it is one mistake made twice rather than a map problem. docs/design-system.md gains the rule, phrased as the check rather than the conclusion: an icon that reads fine on its own can still be wrong beside the one that follows it, so the thing to look at is the bar, not the icon. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y div The load-time placement measured `#rx-log`, which is an empty `<div>` until `createReceptionTicker` writes the card into it: two pixels of border. Both answers that depend on the card's size were therefore wrong on load. - The room test. "Start small where the card would cover the map" was a width test, `max-width: 640px`. A phone held sideways is 844px wide, so it read as a desktop and opened at ten lanes: measured at 844x390, 298px of card over 309px of map, 110% of it, hanging past the bottom edge. It is now the space left under the bar, and the width test stays for the other half of the same question, since below 640px the card is full-bleed. - The clamp. A position remembered from a taller window stood, because the maximum was computed from those two pixels. Seen in the browser at 844x390 restoring y=386: four pixels of card on screen, 294 below the fold. The card at ten lanes is now computed from the geometry tokens instead of measured, which is what makes both decisions independent of when the markup lands. That needed the header's height as a token beside the row pitch, so `--ch-rx-head-h` is declared on both surfaces and pinned equal, the way `--ch-rx-line-h` and `--ch-surface-thin` already are. Verified in the browser at 844x390: was `rx=0,386,0` with a 298px card at 120% of the visible map; now `rx=152,94,b`, the smallest stop, fully on screen. web: 521 unit, e2e green with --fail-on-flaky-tests app: 935 unit, build clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y width Closes efiten#561 ## What it was The bar was `flex-wrap`, so its layout was whatever the flow produced. At 1440 that was two rows, the second one four unrelated readouts on three baselines. At 375 as a guest -- which is what a shared link opens as -- it was **230px over seven rows, 28% of the viewport**, four of those rows carrying no control at all: the guest notice, the heatmap notice, the node counts, the version cluster. And a scan of every leaf under `#bar` for "mesh-hunter" returned nothing. The name lived in `document.title` and the walkthrough, so once that was dismissed the surface people are sent a link to carried no wordmark. ## The shape Brand, one connected group, one primary action, the icon cluster. Not a new design: `#topbar` in the app is this row already and already fits 375px, so the two surfaces now share the bar as well as what is inside their panels. | | before | after | |---|---|---| | bar at 1280 | 81px, two rows | 48px, one row | | bar at 375, guest | 230px, seven rows | 48px, one row | | map behind the bar | 111px at 375 | 0 | | zoom control at 375 | `+` and `−` both unreachable | both reachable | | product name | nowhere | mark + wordmark, mark alone below 640px | `flex-wrap: nowrap` is load-bearing rather than cosmetic. `#map`'s top is measured once (efiten#405), so a bar that grows after that measurement hides the map under it; a bar that cannot wrap cannot grow. ## What does not fit moves, rather than being hidden or copied Below 640px the group keeps `Select target` and `Filters` -- the two the app's own group has carried at that width since efiten#305 -- and `web/barnarrow.js` moves the rest: the time range and the hunter picker into the filter panel, `Start mapping` and `Log in` into the menu. The typed sender prefix and the ignore picker move into the panel at every width; the ignore list comes with the picker, out of the settings sheet, since adding and removing are one job. Moved, not duplicated, for two reasons that are one reason. One filter is one control: a second copy is two things to keep in step. And a hidden control has no box, so `placePopover`, which measures the toggle (efiten#372, efiten#385), has nothing to anchor to. ## Three things the browser said and reading did not - **The notice inherited the bug it was moved out of.** Taken out of the bar and left at the left edge, it landed on Leaflet's zoom control: `elementFromPoint` on the `+` returned `#guest-notice`. Overlays clear the map controls now, and at 375 the zoom control moves to the bottom left, where the full-bleed ticker is not and a thumb is. - **The scrim was under the ticker and the notices.** At z 600 it dimmed the basemap and left both of those lit, which read as the panel being transparent -- the guest notice sits exactly where the open panel's header lands. It is 626 now: over them, still under the bar. - **Restoring by index put the time range back in the wrong place.** Two controls come out of the same group and the second one's index is stale once the first has left, so `tr-wrap` came home after the Filters pill and moved every divider the group draws. It records the parent's original order instead. ## The ticker can be dragged with a thumb Its frame is two 6px strips revealed on `:hover`, and a phone reports `hover: none`, so on touch the one thing the surface rule promises -- that a panel over the map can be moved out of the way -- was not true. The header is the handle there, with a grabber pill, and its own buttons keep taking their taps. ## Register `docs/design-system.md` gains a **Bars** section (one row at every width; what does not fit moves; notices and readouts are not controls and may not take a map control's corner) and a touch rule under Controls: an affordance revealed by hover needs a visible, thumb-sized form under `@media (hover: none)`. ## Tests `e2e/barlayout.spec.js` asserts, as a guest at 375, 768 and 1280: one centre line, `--ch-bar-h` agreeing with the real bar, and both zoom buttons uncovered. Plus where each moved control lands, that each exists exactly once, and that the order comes back when the window grows. `e2e/ticker-place.spec.js` drives the touch drag on a `hasTouch` context. The specs that drove `#f-sender` and `#ig-toggle` straight from the bar now go through the panel, via `fillSender`/`openIgnorePicker` in fixtures. - web: 525 unit, 212 e2e with `--fail-on-flaky-tests` - eslint: clean Verified in the browser at 1280x800 and 375x812, dark and light. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A change a user can see needs an entry in the same PR (AGENTS.md), and on a phone this is the whole top of the screen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Kasper, 1 September. Reversing the touch handle from the commit below, and the reason is better than the one it replaces. Dragging only means something when there is map beside the panel as well as under it. Below 640px the card is full-bleed (`min(680px, 100vw)`), so every position is the same band at a different height: there is no "out of the way" to drag it to. Its stops and its cross are what move it aside there, which is what the app does at every width. So the frame is `display: none` below the breakpoint rather than given a grabber pill, and map.js refuses a drag at that width as well. The strips are 6px of invisible hit area, and an invisible target that does something when pressed by accident is worse than no target. The second guard survived its first mutation, because with the strips hidden nothing ever reaches it. It is now exercised by forcing the frame back on and attempting the drag anyway. `docs/design-system.md` had written the opposite down an hour ago. The surface rule is now about the panel getting out of the way rather than about dragging specifically, and how it does that is a width question, not a touch one. web: 546 unit, 213 e2e with --fail-on-flaky-tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes efiten#564 ## What it was Both surfaces filter the same receptions on the same fields, and presented it differently on every axis. The app opened with two checkboxes and reached the chips third; the map opened with the chips and reached the checkboxes third. One said `Types` where the other said `Traffic types`. The app labelled only its two chip rows and left the checkboxes and the timeframe with no heading at all. Only the map counted what was narrowed, so on the app nothing said a filter was on until you opened the sheet. And the overflow rule was applied to the wrong surface: the map collapsed its chips below 640px, the app showed all fifteen — and the app is always below 640px. Worst of all, "everything" had two representations. The app said it with an explicit `All` chip; the map said it with *no chip lit*, which is correct in the query the row builds and says nothing whatsoever on screen. ## One panel `Time` · `Traffic types` · `Sender id` · `Only show` · `Ignored senders`, in that order, on both. The map adds `Overlays` and `View` after those and only there: they are analysis, and the map is the superset. The app's sheet gains group headings, the count, `Clear N filters`, and the `+N more` collapse. The map's ignore list moves from the settings sheet into the panel — it filters what you see, so it belongs with everything that does — and its two chip rows gain the All chip. ## Two rules, one file each, byte-identical on both surfaces - `chiprow.js` — `nextChipSelection` is what a chip press does, everywhere. The empty Set is All, so the two readings of "no chips active" cannot diverge again: there is one state and the All chip is how it is drawn. Pressing All when it is already on is a no-op, not a toggle that empties the view. `hiddenChipCount` is the `+N` number, computed from the list and the selection rather than measured off the layout — the map counted chips with `offsetWidth === 0`, which answers zero before the first paint. - `barfilters.js` — `activeFilterCount`, now carrying both surfaces' dimensions, so `Clear N filters` and the trigger badge promise the same number on both. `web/parity.test.js` pins both files byte-identical between `web/` and `app/src/`. A byte comparison rather than a behaviour one, because it also catches a comment on one side explaining a rule the other no longer follows. ## The group order is a value, not a template `app/src/filtersheet.js` holds the app sheet's structure, so the thing this issue is about — which groups, in which order, under which words — is something a test reads instead of a string buried in a 2500-line module. `FILTER_GROUPS` is checked against the markup it renders, and `parity.test.js` checks the map's panel against it after removing what is deliberately map-only. Mutation-checked: shuffling the app's order, renaming `Traffic types` back to `Types`, and moving the map's `Ignored senders` after `Overlays` each fail it. ## The All chip never reaches a query It is a drawing of the empty set. `types=all` would be a filter for a packet type that does not exist, and the server buckets these verbatim, so the getters strip it and an e2e pins that neither the URL nor `currentTypes()` ever carries it. The `+N` cap moved from `:nth-child(n+7)` to `n+8` for the same reason: the All chip is the row's first child now, and the old rule would have shown five types and called it six. ## Tests - `chiprow.test.js` — 14 cases, mutation-checked (All toggling instead of resetting, a row that never removes a chip, mutating the caller's Set, an off-by-one cap, counting active chips as hidden). - `app/src/__tests__/filtersheet.test.js` — the group order, one heading per group, the vocabulary, and both rows opening on All. - `e2e/barfilters.spec.js` — the All chip's four behaviours, that it never reaches the query, and that the cap shows six types rather than five plus All. - web: 546 unit, 215 e2e with `--fail-on-flaky-tests` - app: 941 unit, build clean - eslint: clean on both Verified in the browser at 390x844: the sheet's five groups in order, `+8 more` expanding to fifteen, the count and `Clear N filters` tracking each other, and Clear returning both rows to All. **One flake seen and not reproduced:** `auth.spec.js:242` failed once under parallel load and passed on three full re-runs plus in isolation, and passes on the base commit too. Reported rather than smoothed over. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A change a user can see needs an entry in the same PR (AGENTS.md), and this one is the panel they open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two differences the panels only showed once they were put side by side at the same width, which is the whole reason efiten#564 exists. - **The map counted the All chip as a packet type.** "1 of 15" against the app's "1 of 14", because the count read every `.f-chip` in the row and the All chip became one of them. It is a drawing of the empty set, not a type. - **"2 active" against "2 filters".** One number, two words, and the map's own Clear button below it already said "Clear 2 filters". While there, the map's "+N more" now uses `hiddenChipCount` like the app rather than counting chips with `offsetWidth === 0`. That measurement answers zero for every chip before the first paint and while the panel is shut, so the button was silent exactly when it had something to say. Its expanded label is "Show fewer" on both. The app's ignore list gets web/style.css's rules for it: it was unstyled there, a bare browser button and undecorated rows in a panel where everything else had been given a box. - web: 546 unit, 215 e2e with --fail-on-flaky-tests - app: 941 unit, build clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`Clear also clears the hunter selection` emptied its request log just before pressing Clear, then required every entry to carry no `hunter=`. Closing the picker and opening the filter panel can each leave a refresh in flight, and a request issued before Clear still carries the hunter it was issued with — so under parallel load one landed inside that window and failed a button that had worked. Isolated: 0 of 20 alone, about 1 in 3 full runs. It marks the log's position as Clear is pressed and judges only what follows. Not weaker: every post-Clear request must still be clean, which is the window the button actually promises anything about. Mutation-checked against removing `hunterPicker.setSelected([])`. web: 216 e2e green four runs in a row with --fail-on-flaky-tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
The red app check is a known flake, and not from this branch
Measured on this branch: 3 failures in 20 runs of that test alone, so roughly 15% per CI run The mechanism is in Left alone deliberately rather than folded in here — it is a second subject, and this PR is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #564
What it was
Both surfaces filter the same receptions on the same fields, and presented it differently on
every axis. The app opened with two checkboxes and reached the chips third; the map opened with
the chips and reached the checkboxes third. One said
Typeswhere the other saidTraffic types. The app labelled only its two chip rows, leaving the checkboxes and thetimeframe with no heading at all. Only the map counted what was narrowed, so on the app nothing
said a filter was on until you opened the sheet. And the overflow rule was applied to the wrong
surface: the map collapsed its chips below 640px, the app showed all fifteen — and the app is
always below 640px.
Worst of all, "everything" had two representations. The app said it with an explicit
Allchip; the map said it with no chip lit, which is correct in the query the row builds and says
nothing whatsoever on screen.
One panel
Time·Traffic types·Sender id·Only show·Ignored senders, in that order, onboth. The map adds
OverlaysandViewafter those and only there: they are analysis, and themap is the superset.
The app's sheet gains group headings, the count,
Clear N filters, and the+N morecollapse.The map's two chip rows gain the All chip.
Two rules, one file each, byte-identical on both surfaces
chiprow.js—nextChipSelectionis what a chip press does, everywhere. The empty Setis All, so the two readings of "no chips active" cannot diverge again: there is one state,
and the All chip is how it is drawn. Pressing All when it is already on is a no-op, not a
toggle that empties the view.
hiddenChipCountis the+Nnumber, computed from the list andthe selection rather than measured off the layout — the map counted chips with
offsetWidth === 0, which answers zero for every chip before the first paint and while thepanel is shut, so the button was silent exactly when it had something to say.
barfilters.js—activeFilterCount, now carrying both surfaces' dimensions, so thetrigger badge and
Clear N filterspromise the same number on both.web/parity.test.jspins both files byte-identical betweenweb/andapp/src/. A bytecomparison rather than a behaviour one, because it also catches a comment on one side explaining
a rule the other no longer follows.
The group order is a value, not a template
app/src/filtersheet.jsholds the app sheet's structure, so the thing this issue is about —which groups, in which order, under which words — is something a test reads instead of a string
buried in a 2500-line module.
FILTER_GROUPSis checked against the markup it renders, andparity.test.jschecks the map's panel against it after removing what is deliberately map-only.Mutation-checked: shuffling the app's order, renaming
Traffic typesback toTypes, andmoving the map's
Ignored sendersafterOverlayseach fail it.Three things that only showed up side by side
The two panels were opened at the same width and photographed next to each other, which is what
this issue asks for and what none of the tests can do:
It is a drawing of the empty set, not a type.
below it already said "Clear 2 filters".
where everything else had been given a box. It takes
web/style.css's rules for it now.The All chip never reaches a query
types=allwould be a filter for a packet type that does not exist, and the server bucketsthese verbatim, so the getters strip it and an e2e pins that neither the URL nor
currentTypes()ever carries it. The+Ncap moved from:nth-child(n+7)ton+8for thesame reason: the All chip is the row's first child now, and the old rule would have shown five
types and called it six.
A flake fixed rather than reported
hunterpicker.spec.js's "Clear also clears the hunter selection" emptied its request log justbefore pressing Clear, then required every entry to carry no
hunter=. Closing the picker andopening the filter panel can each leave a refresh in flight, and a request issued before Clear
still carries the hunter it was issued with — so under parallel load one landed inside that
window and failed a button that had worked. Measured: 0 of 20 in isolation, about 1 in 3 full
runs.
It marks the log's position as Clear is pressed and judges only what follows. Not weaker: every
post-Clear request must still be clean, which is the window the button actually promises
anything about. Mutation-checked against removing
hunterPicker.setSelected([]).Tests
chiprow.test.js— 14 cases, mutation-checked (All toggling instead of resetting, a row thatnever removes a chip, mutating the caller's Set, an off-by-one cap, counting active chips as
hidden).
app/src/__tests__/filtersheet.test.js— the group order, one heading per group, thevocabulary, and both rows opening on All.
e2e/barfilters.spec.js— the All chip's four behaviours, that it never reaches the query,that the cap shows six types rather than five plus All, and that both surfaces say
2 filtersand1 of 14.web: 546 unit, 216 e2e with
--fail-on-flaky-tests, green four runs in a rowapp: 941 unit, build clean
eslint: clean on both
Verified in the browser at 390x844 on both surfaces, side by side: the five groups in order,
+8 moreexpanding to fifteen, the count andClear N filterstracking each other, and Clearreturning both rows to All.
Deliberate, and worth knowing
Sender unknownstays on both. app,web: Sender unknown and the Unnamed chip are two controls for one filter #535 removes it as a duplicate of theUnnamedchip andhas not landed; this issue's note says landing it first would take one row off both panels.
activeFilterCount. It is a barcontrol that travels in the URL, and Clear has never reset it — the app's
Plot lastiscounted, because Clear does reset that.
🤖 Generated with Claude Code