Fix duplicate, unclickable row when re-dropping the same track - #3
Open
shyam-habarakada wants to merge 1 commit into
Open
Fix duplicate, unclickable row when re-dropping the same track#3shyam-habarakada wants to merge 1 commit into
shyam-habarakada wants to merge 1 commit into
Conversation
Dropping a track that was already separated added a second row to the "Separated" list instead of updating the existing one, and the new row was stuck: hovering and clicking did nothing until the pointer moved over the older row instead. Both symptoms traced back to the same two spots: - Drops::accept always inserted a new row at the top of the list with no check for whether that file was already there, so re-running a track duplicated it rather than refreshing it. - Each row's click/hover handling was keyed off the track's displayed title. Two rows for the same file have the same title, so they ended up sharing one interaction id, and egui attributed input to only one of them. Fixed both: re-dropping a track now replaces its existing row and moves it to the top instead of adding a new one, and each row's id is now derived from the source file path rather than the title, so two different files that happen to share a name can never collide either. Added a test (re_dropping_a_track_replaces_its_row_instead_of_duplicating) that drops the same path twice and checks the list stays deduplicated, with the re-run at the top and unrelated rows undisturbed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
nsaintot
requested changes
Sep 6, 2026
Comment on lines
167
to
168
| /// different folders are two drops, and dropping one track twice makes two | ||
| /// rows that must be dismissable separately. |
Owner
There was a problem hiding this comment.
Comment is stale with the dedup. Trim or update
| recent.insert(0, Arc::clone(&dropped)); | ||
| recent.truncate(REMEMBERED); | ||
| } | ||
| remember(&mut self.recent.lock(), Arc::clone(&dropped)); |
Owner
There was a problem hiding this comment.
[Nit] Dedup lives only in accept; nothing structural stops a future insert from bypassing remember.
I would tend to favor a method for remember rather than a free function that only has one caller.
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
2 participants
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.
Fixes #2
The bug
If you drop the same track on the window twice (or drop it again after it already finished separating), the "Separated" list ended up with two rows for it instead of one. The newer of the two rows was broken: hovering and clicking did nothing on it. To get any hover/click feedback on that track, you had to move the mouse onto the older duplicate row further down the list instead.
Why it happened
Two separate things combined to cause this:
Nothing de-duplicated the list. Every time a file was dropped, the window just inserted a brand-new row at the top of the "recent drops" list, with no check for whether that same file was already sitting somewhere in the list. So re-running a track always added a second row rather than updating the one already there.
Rows were sharing an identity behind the scenes. Each row's click/hover handling needs a stable ID so the UI toolkit (egui) can tell "this row was pressed" from "that row was pressed." That ID was built from the track's display name (the file's title). But two rows for the same file obviously have the same title, so both rows ended up using the exact same ID. egui only routes input correctly when every interactive element has a unique ID, so with two rows sharing one ID, input got misattributed — the newer row looked normal but silently couldn't be clicked, while the older row (usually hidden further down the list) picked up the intended interaction instead.
The fix
Tests added
Added a test that simulates dropping the same file twice: it drops file A, then drops an unrelated file B, then drops file A again, and checks that:
All existing tests continue to pass.
🤖 Generated with Claude Code