feat(ui): add persisted sound sorting - #211
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesSound sorting and indexed rendering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant HonkHonk
participant FilterCache
participant SoundGrid
User->>Header: choose sort key or direction
Header->>HonkHonk: send sort message
HonkHonk->>FilterCache: refresh filtered sound indices
FilterCache-->>HonkHonk: ordered indices
HonkHonk->>SoundGrid: render sounds with indices
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Add reusable list sort controls, tolerant per-view preferences, and deterministic ordering for the main sound grid. Co-Authored-By: Codex <noreply@openai.com>
Deserialize sort preferences independently so malformed future entries fall back without invalidating otherwise usable application settings. Co-Authored-By: Codex <noreply@openai.com>
Move sound filtering and sorting to update-side invalidation boundaries so Iced view construction only indexes cached results. Project only the active sort key and keep render benchmarks on the production cached-index API. Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/sound_grid.rs (1)
87-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPadding count should be based on resolved tiles, not raw index-chunk length.
missing_tile_slots(chunk.len(), columns)pads using the chunk's raw index count, buttilesmay have fewer elements thanchunk.len()if any index inchunkfails to resolve viasounds.get(index)(dropped byfilter_map). In that case the row ends up short by the number of dropped indices instead of being padded up tocolumns, causing a visually misaligned row rather than a full, correctly-padded one.🐛 Proposed fix
- tiles.extend((0..missing_tile_slots(chunk.len(), columns)).map(|_| { + tiles.extend((0..missing_tile_slots(tiles.len(), columns)).map(|_| { Space::new() .width(Length::Fill) .height(tile_layout::tile_slot_height()) .into() }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/sound_grid.rs` around lines 87 - 109, Update the padding calculation in the row-building closure around sound_tile::view so missing_tile_slots uses the number of resolved tiles in tiles, not chunk.len(). Preserve the existing padding behavior while ensuring rows with unresolved sounds are filled to columns.
🧹 Nitpick comments (1)
src/state/config/persistence.rs (1)
19-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate
load()/load_from().Both methods repeat the same exists-check → default-save → read → deserialize sequence, differing only in how
pathis obtained.♻️ Proposed refactor
pub fn load() -> Result<Self, ConfigError> { - let path = Self::config_path()?; - - if !path.exists() { - let config = Self::default(); - config.save()?; - return Ok(config); - } - - let contents = read_config(&path)?; - serde_json::from_str(&contents).map_err(|source| ConfigError::Deserialize { - path: path.display().to_string(), - source, - }) + Self::load_from(&Self::config_path()?) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/state/config/persistence.rs` around lines 19 - 53, Deduplicate the shared loading logic by making load() obtain Self::config_path() and delegate to load_from(), leaving load_from() as the single implementation of the exists-check, default-save, read, and deserialization sequence. Preserve the existing ConfigError handling and behavior for both configured and explicit paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/ui/sound_grid.rs`:
- Around line 87-109: Update the padding calculation in the row-building closure
around sound_tile::view so missing_tile_slots uses the number of resolved tiles
in tiles, not chunk.len(). Preserve the existing padding behavior while ensuring
rows with unresolved sounds are filled to columns.
---
Nitpick comments:
In `@src/state/config/persistence.rs`:
- Around line 19-53: Deduplicate the shared loading logic by making load()
obtain Self::config_path() and delegate to load_from(), leaving load_from() as
the single implementation of the exists-check, default-save, read, and
deserialization sequence. Preserve the existing ConfigError handling and
behavior for both configured and explicit paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7de4d802-062f-4a08-b8aa-f014b31804df
📒 Files selected for processing (21)
benches/grid_render.rsbenches/support/mod.rsdocs/superpowers/specs/2026-07-23-issue-197-design.mdsrc/app/filtering.rssrc/app/filtering/cache.rssrc/app/header.rssrc/app/library_scan.rssrc/app/mod.rssrc/app/sorting.rssrc/app/sorting/tests.rssrc/app/sorting/view.rssrc/app/sound_metadata.rssrc/state/config.rssrc/state/config/persistence.rssrc/state/config/sort.rssrc/state/config/types.rssrc/state/mod.rssrc/ui/list_controls/filter.rssrc/ui/list_controls/mod.rssrc/ui/list_controls/sort.rssrc/ui/sound_grid.rs
Keep grid rows aligned when cached indices no longer resolve and consolidate config loading through the explicit-path implementation. Co-Authored-By: Codex <noreply@openai.com>
Why
Sound libraries need predictable ordering that survives restarts without coupling sort preferences to the session-only filter.
What
Add reusable sort state, chip, and dismissible options menu; persist tolerant per-view preferences; and sort main sound tiles by Name, Length, Folder, Modified, or Added with deterministic ties and unknown dates last. Malformed sort preference fields or future view entries now fall back independently without weakening validation for the rest of the application config. The change also extracts header and config helpers so touched files remain within the repository size limit.
Testing
cargo test(654 library tests)cargo test(all binary/integration/doc targets)cargo clippy --all-targets -- -D warningscargo fmt --all -- --check🤖 Co-authored by Codex. Closes #197.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation