Skip to content

Conversation

krischerven
Copy link
Contributor

Currently, decks can be drag-and-dropped to their parent, which does nothing (from the user's perspective), but claims to rename the deck. This fixes that behavior.

@dae
Copy link
Member

dae commented Mar 15, 2025

The correct way to fix this would be to have reparent_decks() in the Rust layer not include such decks in the returned change count.

@krischerven krischerven force-pushed the fix-duplicate-deck-drag-drop branch from 3e1dfa2 to fd3cbd2 Compare March 17, 2025 22:05
@krischerven krischerven force-pushed the fix-duplicate-deck-drag-drop branch from fd3cbd2 to 01e22a2 Compare March 17, 2025 22:08
@krischerven
Copy link
Contributor Author

The correct way to fix this would be to have reparent_decks() in the Rust layer not include such decks in the returned change count.

Alright, I've implemented that solution.

@dae
Copy link
Member

dae commented Mar 19, 2025

I'm afraid I think this is still needlessly complicated. I suspect all that is required is

diff --git a/rslib/src/decks/reparent.rs b/rslib/src/decks/reparent.rs
index a9fcd318f..bfb3fee05 100644
--- a/rslib/src/decks/reparent.rs
+++ b/rslib/src/decks/reparent.rs
@@ -36,6 +36,9 @@ impl Collection {
         for deck in deck_ids {
             if let Some(mut deck) = self.storage.get_deck(*deck)? {
                 if let Some(new_name) = deck.name.reparented_name(target_name) {
+                    if new_name == deck.name {
+                        continue;
+                    }
                     count += 1;
                     let orig = deck.clone();

Also, I'm not sure we should be hiding the message in that case. The user has performed an action - wouldn't it be better to show them '0 decks changed' (i.e. "I received your request, but it doesn't make sense"), rather than not showing anything, which leaves the user wondering whether Anki received the user's request or not.

@dae
Copy link
Member

dae commented Mar 19, 2025

Tags probably need similar treatment as well.

@krischerven
Copy link
Contributor Author

Also, I'm not sure we should be hiding the message in that case. The user has performed an action - wouldn't it be better to show them '0 decks changed' (i.e. "I received your request, but it doesn't make sense"), rather than not showing anything, which leaves the user wondering whether Anki received the user's request or not.

Yeah, I suppose that's fair.

@krischerven
Copy link
Contributor Author

@dae I implemented your solution. What needs to be changed wrt tags? It seems that I can't reparent them in the first place.

@dae
Copy link
Member

dae commented Mar 24, 2025

Thanks, looks good so far. Tags can be reparented in the same way is decks in the browser, by switching the sidebar to select mode, then dragging.

@krischerven
Copy link
Contributor Author

Same concept implemented for tags now.

@dae
Copy link
Member

dae commented Mar 26, 2025

Sorry to drag this out, but I think this could be done a bit more efficiently by adjusting reparented_name() to return None if the old and new names match. Then the map doesn't need to be iterated again, and the changes are minimized.

@krischerven
Copy link
Contributor Author

krischerven commented Mar 28, 2025

Thanks, I didn't realize we were filtering out the Nones there! Should be good to go now

@dae dae merged commit 52781aa into ankitects:main Mar 31, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants