From 1ece868d029bb7ed360b07df5434d915957e63b4 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 6 Apr 2021 11:18:13 +1000 Subject: [PATCH] shift keep-current-selection logic into sidebar's refresh() By calling refresh() manually after performing an op, we were refreshing twice, and the selection was being lost when changes were made outside of the sidebar. Also drop the after_hooks arg to perform_op(), since nothing is using it now. --- qt/aqt/main.py | 10 +------ qt/aqt/operations/deck.py | 5 ++-- qt/aqt/operations/tag.py | 4 +-- qt/aqt/sidebar.py | 59 ++++++++++++++++++++++----------------- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/qt/aqt/main.py b/qt/aqt/main.py index 7090c4a6b..be5f4bf7a 100644 --- a/qt/aqt/main.py +++ b/qt/aqt/main.py @@ -771,7 +771,6 @@ class AnkiQt(QMainWindow): *, success: PerformOpOptionalSuccessCallback = None, failure: PerformOpOptionalFailureCallback = None, - after_hooks: Optional[Callable[[], None]] = None, handler: Optional[object] = None, ) -> None: """Run the provided operation on a background thread. @@ -795,10 +794,6 @@ class AnkiQt(QMainWindow): If op() throws an exception, it will be shown in a popup, or passed to failure() if it is provided. - - after_hooks() will be called after hooks are fired, if it is provided. - Components can use this to ignore change notices generated by operations - they invoke themselves, or perform some subsequent action. """ self._increase_background_ops() @@ -826,7 +821,7 @@ class AnkiQt(QMainWindow): status = self.col.undo_status() self._update_undo_actions_for_status_and_save(status) # fire change hooks - self._fire_change_hooks_after_op_performed(result, after_hooks, handler) + self._fire_change_hooks_after_op_performed(result, handler) self.taskman.with_progress(op, wrapped_done) @@ -844,7 +839,6 @@ class AnkiQt(QMainWindow): def _fire_change_hooks_after_op_performed( self, result: ResultWithChanges, - after_hooks: Optional[Callable[[], None]], handler: Optional[object], ) -> None: if isinstance(result, OpChanges): @@ -859,8 +853,6 @@ class AnkiQt(QMainWindow): # fire legacy hook so old code notices changes if self.col.op_made_changes(changes): gui_hooks.state_did_reset() - if after_hooks: - after_hooks() def _synthesize_op_did_execute_from_reset(self) -> None: """Fire the `operation_did_execute` hook with everything marked as changed, diff --git a/qt/aqt/operations/deck.py b/qt/aqt/operations/deck.py index fc6ca6250..4081626cf 100644 --- a/qt/aqt/operations/deck.py +++ b/qt/aqt/operations/deck.py @@ -3,7 +3,7 @@ from __future__ import annotations -from typing import Callable, Optional, Sequence +from typing import Optional, Sequence from anki.decks import DeckCollapseScope, DeckId from aqt import AnkiQt, QWidget @@ -41,10 +41,9 @@ def rename_deck( mw: AnkiQt, deck_id: DeckId, new_name: str, - after_rename: Callable[[], None] = None, ) -> None: mw.perform_op( - lambda: mw.col.decks.rename(deck_id, new_name), after_hooks=after_rename + lambda: mw.col.decks.rename(deck_id, new_name), ) diff --git a/qt/aqt/operations/tag.py b/qt/aqt/operations/tag.py index 49b38cb2c..0f22ff4cb 100644 --- a/qt/aqt/operations/tag.py +++ b/qt/aqt/operations/tag.py @@ -3,7 +3,7 @@ from __future__ import annotations -from typing import Callable, Optional, Sequence +from typing import Optional, Sequence from anki.collection import OpChangesWithCount from anki.notes import NoteId @@ -57,7 +57,6 @@ def rename_tag( parent: QWidget, current_name: str, new_name: str, - after_rename: Callable[[], None], ) -> None: def success(out: OpChangesWithCount) -> None: if out.count: @@ -68,7 +67,6 @@ def rename_tag( mw.perform_op( lambda: mw.col.tags.rename(old=current_name, new=new_name), success=success, - after_hooks=after_rename, ) diff --git a/qt/aqt/sidebar.py b/qt/aqt/sidebar.py index e9b9b1d1c..22bec6766 100644 --- a/qt/aqt/sidebar.py +++ b/qt/aqt/sidebar.py @@ -172,6 +172,18 @@ class SidebarItem: ) return self._search_matches_self or self._search_matches_child + def has_same_id(self, other: SidebarItem) -> bool: + "True if `other` is same type, with same id/name." + if other.item_type == self.item_type: + if self.item_type == SidebarItemType.TAG: + return self.full_name == other.full_name + elif self.item_type == SidebarItemType.SAVED_SEARCH: + return self.name == other.name + else: + return other.id == self.id + + return False + class SidebarModel(QAbstractItemModel): def __init__(self, sidebar: SidebarTreeView, root: SidebarItem) -> None: @@ -432,13 +444,16 @@ class SidebarTreeView(QTreeView): self.refresh() self._refresh_needed = False - def refresh( - self, is_current: Optional[Callable[[SidebarItem], bool]] = None - ) -> None: + def refresh(self) -> None: "Refresh list. No-op if sidebar is not visible." if not self.isVisible(): return + if self.model() and (idx := self.currentIndex()): + current_item = self.model().item_for_index(idx) + else: + current_item = None + def on_done(root: SidebarItem) -> None: # user may have closed browser if sip.isdeleted(self): @@ -454,8 +469,8 @@ class SidebarTreeView(QTreeView): self.search_for(self.current_search) else: self._expand_where_necessary(model) - if is_current: - self.restore_current(is_current) + if current_item: + self.restore_current(current_item) self.setUpdatesEnabled(True) @@ -464,8 +479,8 @@ class SidebarTreeView(QTreeView): self.mw.query_op(self._root_tree, success=on_done) - def restore_current(self, is_current: Callable[[SidebarItem], bool]) -> None: - if current := self.find_item(is_current): + def restore_current(self, current: SidebarItem) -> None: + if current := self.find_item(current.has_same_id): index = self.model().index_for_item(current) self.selectionModel().setCurrentIndex( index, QItemSelectionModel.SelectCurrent @@ -1165,21 +1180,21 @@ class SidebarTreeView(QTreeView): def rename_deck(self, item: SidebarItem, new_name: str) -> None: if not new_name: return - new_name = item.name_prefix + new_name + + # update UI immediately, to avoid redraw + item.name = new_name + + full_name = item.name_prefix + new_name deck_id = DeckId(item.id) def after_fetch(deck: Deck) -> None: - if new_name == deck.name: + if full_name == deck.name: return rename_deck( mw=self.mw, deck_id=deck_id, - new_name=new_name, - after_rename=lambda: self.refresh( - lambda other: other.item_type == SidebarItemType.DECK - and other.id == item.id - ), + new_name=full_name, ) self.mw.query_op(lambda: self.mw.col.get_deck(deck_id), success=after_fetch) @@ -1208,16 +1223,13 @@ class SidebarTreeView(QTreeView): new_name = item.name_prefix + new_name item.name = new_name_base + item.full_name = new_name rename_tag( mw=self.mw, parent=self.browser, current_name=old_name, new_name=new_name, - after_rename=lambda: self.refresh( - lambda item: item.item_type == SidebarItemType.TAG - and item.full_name == new_name - ), ) # Saved searches @@ -1250,10 +1262,7 @@ class SidebarTreeView(QTreeView): return conf[name] = search self._set_saved_searches(conf) - self.refresh( - lambda item: item.item_type == SidebarItemType.SAVED_SEARCH - and item.name == name - ) + self.refresh() def remove_saved_searches(self, _item: SidebarItem) -> None: selected = self._selected_saved_searches() @@ -1277,10 +1286,8 @@ class SidebarTreeView(QTreeView): conf[new_name] = filt del conf[old_name] self._set_saved_searches(conf) - self.refresh( - lambda item: item.item_type == SidebarItemType.SAVED_SEARCH - and item.name == new_name - ) + item.name = new_name + self.refresh() def save_current_search(self) -> None: if (search := self._get_current_search()) is None: