feat(ui): wire filter + sort into the slots view by thewrz · Pull Request #242 · wrzonance/HonkHonk

thewrz · 2026-08-01T20:57:20Z

Why

The slot manager grid had no way to search or sort 20 sound/macro slots — the
hotkey panel already got a search bar + sort chip in #227, and slots was the
last major view without one. Closes #198.

What

Wires the same filter/sort infrastructure the hotkey panel uses (#227) into
the slot manager grid:

New SlotRow projection (app/slots/rows.rs) flattens each of the 20 slots
(sound, macro, or empty) into a searchable/sortable row — name, filename,
tag, duration, modified/added timestamps.
SlotSortKey (already defined for feat(ui): add hotkey sort/filter search bar with settings-search extraction #227, previously #[allow(dead_code)])
is now a live second consumer via SortKey<SlotRow>.
FilterTarget::Slots routes type-to-filter and Escape-to-dismiss into the
slot manager's own FilterState/sort menu, independent of the hotkey and
settings filters.
New "Search slots…" search bar + sort chip in the slot manager header,
built from the same search_bar.rs widgets as hotkeys/settings.
view_slot_manager now self-layers its sort overlay (mirroring
view_settings) instead of the outer view() doing it, and the grid
renders slots in filtered+sorted order via a render_order: &[u8]
parameter rather than mutating SlotManagerCtx.
Empty search results show "No slots match your search." instead of blank
tiles.

Design decisions

SlotRow carries no placeholder/has-content flag. Blank strings and
None for an empty slot already route correctly through filter_items
and SortKey::value_unknown, so no extra field was needed.
slot_filter/slot_sort are wired live in this PR, not deferred.
Unlike the hotkey fields when they first landed, these get no
#[allow(dead_code)] — slots.rs and the new search bar/view consume
them immediately.
SlotManagerCtx stays unchanged. Row order flows through an
independently-lifetimed render_order: &[u8] function parameter rather
than a new ctx field — confirmed via spike that only u8 copies need to
flow into the returned Element, so no lifetime coupling was introduced.
Tie-break is always slot index ascending, regardless of the active
sort key, so re-sorting never visually shuffles ties.
handle_escape needed no Slots arm. The sort-menu-anchor field is
already shared across every view, so the existing dismiss path covers the
slot manager for free.

Testing

cargo fmt --check
cargo clippy --all-targets -- -D warnings
cargo test (799 unit tests + all integration suites green)
cargo tarpaulin (diagnostic; not installed in this environment —
runs in CI)
Manual verification: search/sort in the running app

🤖 Co-authored by Claude Sonnet 5. Closes #198.

Adds FilterTarget::Slots so the slot manager owns its own type-to-filter and Escape-clearing routing, mirroring the Settings > Shortcuts bindings list added in #199. active_filter_target now resolves Some(Slots) whenever ViewMode::SlotManager is active, replacing the prior "slot manager has no filter surface" None. Wires the new target through filter_input_id, filter_state_mut, and handle_type_to_filter, backed by a new slot_filter: FilterState field on HonkHonk and a dedicated slots_input_id() in ui/search_bar.rs. The slot manager's own search bar UI lands in a follow-up task in this issue's chain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Adds the slot manager grid's pure row-building layer: SlotRow, a fully owned 20-row snapshot of every fixed slot in slot order, and the SortKey<SlotRow> impl for the shared SlotSortKey. Unlike the Settings bindings list, row membership is total (one row per slot, not per bound trigger), and dangling sound/macro references collapse to the same fully-blank haystack as an empty slot rather than a placeholder label, so filter/sort logic needs no separate has-content flag. Pins the sort-reorders-render-only invariant first (the single most important test per the issue): sorting never changes row membership, cardinality, or content, only order. Also covers total/stable row count, the blank haystack for dangling/empty slots, and SlotSortKey::SlotNumber's always-false value_unknown with its always-ascending tie_break. Wiring into HonkHonk::slot_rows()/slot_render_order() and the rest of the slot manager view lands in follow-up tasks in this issue's chain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…#198) Extends app/slots.rs with the slot manager's own SlotSortState (mirroring hotkeys::HotkeySortState), sort_prefs["slots"] round-trip loading, and the HonkHonk-level query surface: slot_rows() composes filter_items() with SlotSortState::sorted() (filter narrows before sort reorders), plus slot_render_order()/slot_filter_query()/slot_sort_state() accessors. Adds the message-driven mutators (toggle/select/dismiss the sort menu, persist the preference, replace the filter query) and wires them through five new Message variants (message.rs) and update() arms (mod.rs) so the new slot_filter/slot_sort state is genuinely live rather than deferred — mirroring the existing Hotkey* plumbing exactly. The slot manager's own search bar and sort-menu UI land in follow-up tasks in this issue's chain. Pins three invariants: filtering always narrows before sorting reorders, filter matching is a case-insensitive substring over exactly [display_name, filename, tag], and sort preference persistence round-trips (with safe fallback to the default for missing/unknown/corrupt config). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Adds view_slot_sort_overlay, mirroring hotkeys::view_hotkey_sort_overlay but keyed on ViewMode::SlotManager instead of SettingsSection so a stale sort_menu_anchor from another view never leaks an overlay onto the slot manager. Wiring into ui::slot_manager::view_slot_manager follows in a later task in this issue's chain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The Slot* Message variants, HonkHonk struct fields, constructor wiring, and update() dispatch arms for the slot manager's filter/sort surface were already delivered by prior commits in this issue's task chain (15d1f21, 9a0c108, 952d506) and verified still green here. What was missing was a test pinning the actual invariant at the Message boundary: dispatching Message::TypeToFilter while view_mode == ViewMode::SlotManager must land in slot_filter (not the tiles filter), schedule a focus task for the slot manager's own search input, and never fire in Settings' default (non-Hotkeys) section. Adds typed_filter_text_routes_to_slots_when_slot_manager_is_active and extends the two existing cross-target focus/id assertions to cover Slots alongside Tiles/Hotkeys. Splits filtering/tests.rs's tiles-grid filter/sort tests into filtering/grid_tests.rs to keep both files under the repo's 400-line cap after the additions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Adds view_slots_search_bar(), mirroring the hotkeys search bar exactly (placeholder "Search slots…", stable slots_input_id() widget id already introduced in 15d1f21). Pins the widget-build invariant for empty and populated queries, matching the hotkeys test pair. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Wires the slot manager's query surface (slot_render_order, slot_filter_query, slot_sort_state, view_slot_sort_overlay) into view_slot_manager: a search bar and sort chip in the header, a self-layered Stack overlay for the sort menu (mirroring view_settings's #112 pattern), and a grid that lays tiles out in render_order rather than a fixed slot-index sweep — reordering the grid without ever remapping a tile's own slot-number identity. Removes the now-stale #[allow(dead_code)] left over from the earlier tasks in this chain now that every query-surface item is reachable from production code, and scopes SlotSortKey::from_id's dead_code allow to non-test builds only (it remains genuinely unreachable outside its own round-trip test, by design — each consumer module does its own ALL-scan instead of calling it directly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

coderabbitai · 2026-08-01T20:57:26Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ee99ca0-3129-421b-8efb-428646256824

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

_{Comment @coderabbitai help to get the list of available commands.}

SlotSortKey::value_unknown() returned false for Name and Tag, so a dangling or empty slot (blank display_name/tag) sorted by plain string comparison instead of landing last regardless of Direction as the pinned invariant requires -- its position flipped between ascending and descending. Fixed by treating a blank display_name/tag as unknown, mirroring how Length/Modified/Added already treat their Option fields. Also strengthens the composed-API cardinality invariant test (every slot index appears exactly once in slot_render_order() under an empty query, for every sort key/direction) at the actual HonkHonk::slot_render_order() level, not just via the disconnected rows::build_slot_rows + SortState::sorted primitives. src/app/mod.rs already carries a documented, untested-by-design known violation of the project's 400-line file cap; rather than compound it further inside this fix, the compounding-growth concern is tracked in #243 and the AGENTS.md note now points there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

overlay_hidden_without_an_anchor never exercised the (SlotManager, anchor=None) state — HonkHonk::new_for_test() defaults to ViewMode::Main, so its setup assertion passed on the view_mode disjunct alone and the anchor guard (self.sort_menu_anchor?) went unpinned. Add a test that sets view_mode = SlotManager with no anchor so a regression dropping that guard (e.g. unwrap_or(cursor_pos)) fails the suite instead of shipping silently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…/sort Codex (gpt-5.6-sol, xhigh) reviewed the branch as the draft-phase adversarial gate and raised three findings, all reproduced and confirmed before fixing. Serialize simulator-based tests (P1). `iced_test::simulator` drives iced's process-global font/text state, which is not safe to build from two threads at once. The four new `slot_manager::grid_tests` hung on 3 of 20 parallel runs of `cargo test --lib ui::slot_manager::grid_tests`, while 20 runs under `--test-threads=1` passed; the pre-existing settings simulator tests were clean over 20 runs, so the hazard arrived with these tests. A new test-only `crate::test_lock::gui_lock()` serializes every simulator site (the new grid tests and the existing settings harness) while leaving the other ~850 tests parallel. After the fix: 25/25 parallel runs clean. Pad partial grid rows (P2). Filtering can now end the grid on a short row, which the fixed 20-slot grid could never do before. Tiles are `Length::Fill`, so a lone match stretched across the whole grid as one full-width card. Rows now pad to five with filler spaces, mirroring `sound_grid`. Name unnamed macros consistently (P2). `MacroStore` accepts a blank or whitespace-only name, which every rendering surface shows as "Untitled macro" via `slot_manager::display_name`. The row model derived its own name instead, so the grid was searchable and sortable by a value the user cannot see: querying the visible label matched nothing, and a whitespace-only name sorted ahead of every real one. Rows now go through the same helper. `src/app/mod.rs` is deliberately untouched here; its split is tracked in #243. Refs #198. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

thewrz · 2026-08-01T22:17:25Z

This was written agentically; verify its assertions and edit accordingly:

Adversarial review (draft-phase gate)

Reviewer: Codex gpt-5.6-sol, reasoning effort xhigh (cross-harness), run once against origin/main...HEAD after CI went green. 3 findings, 3 confirmed, 3 fixed in 427f297.

[P1] Simulator tests are not safe to run in parallel — FIXED

iced_test::simulator drives iced's process-global font/text state, so two simulators built concurrently can wedge the test binary.

Reproduced before fixing:

run mode	result
`cargo test --lib ui::slot_manager::grid_tests` (parallel)	3 hangs / 20 runs
same, `--test-threads=1`	20/20 pass
pre-existing `app::settings::gui_tests` (parallel)	20/20 pass
full `cargo test`	8/8 pass

So the hazard arrived with this PR's four simulator tests rather than being a latent property of the existing harness — and the full-suite runs that CI actually executes were passing by luck, not by construction.

Fix: a test-only crate::test_lock::gui_lock() (src/test_lock.rs) that every simulator site takes for its whole body — the four new grid tests and the five pre-existing settings/test_support.rs sites. The remaining ~850 tests still run in parallel. Poisoning is recovered from rather than propagated, so one failing test cannot cascade. After the fix: 25/25 parallel runs clean.

Codex reported this as a reproducible SIGSEGV; what I could reproduce here was an intermittent hang. Same root cause and same fix, but the severity is intermittent, not deterministic.

[P2] Filtered grid stretched a partial row — FIXED

Filtering can now leave 1–4 tiles in the last row, which the fixed 20-slot grid could never do before this PR. Tiles are Length::Fill, so a single match rendered as one full-width card. Rows now pad to five with filler spaces, mirroring sound_grid::missing_tile_slots; pinned by incomplete_rows_reserve_all_missing_tile_slots, matching how sound_grid pins the same contract.

[P2] Unnamed macros were searchable only by an invisible name — FIXED

MacroStore accepts a blank or whitespace-only name, which every rendering surface shows as "Untitled macro" via slot_manager::display_name (the helper added by the #169 review precisely so all surfaces agree). The row model derived its own name instead, so the grid was searchable and sortable by a value the user cannot see: searching the visible label matched nothing, and a whitespace-only name sorted ahead of every real one instead of being treated as unnamed. Rows now go through the same helper. Pinned by unnamed_macro_slots_filter_and_sort_by_their_visible_label, verified red before the fix and green after.

Not addressed here

src/app/mod.rs remains untouched by these fixes; its 400-line-cap violation and split are tracked in #243, not this PR.

Verification

cargo fmt --check clean · cargo clippy --all-targets -- -D warnings clean · cargo test 859 passed, 0 failed. The PR's core invariant test sort_reorders_render_only (sorting reorders rendering only; it never remaps which sound lives in which slot) was not modified and still passes.

🤖 Co-authored by Claude Opus 5.

thewrz and others added 7 commits August 1, 2026 13:02

thewrz and others added 3 commits August 1, 2026 14:11

Navigation Menu

Uh oh!

feat(ui): wire filter + sort into the slots view - #242

feat(ui): wire filter + sort into the slots view#242
thewrz wants to merge 10 commits into
mainfrom
feat/issue-198

thewrz commented Aug 1, 2026

Uh oh!

coderabbitai Bot commented Aug 1, 2026 •

edited

Loading

Review skipped

Uh oh!

thewrz commented Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thewrz commented Aug 1, 2026

Why

What

Design decisions

Testing

Uh oh!

coderabbitai Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

thewrz commented Aug 1, 2026

Adversarial review (draft-phase gate)

[P1] Simulator tests are not safe to run in parallel — FIXED

[P2] Filtered grid stretched a partial row — FIXED

[P2] Unnamed macros were searchable only by an invisible name — FIXED

Not addressed here

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Aug 1, 2026 •

edited

Loading