From 18d2d3c1a4ca1761cabff2dfafdcc5b9d7e1696b Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 25 Oct 2022 00:45:31 -0700 Subject: [PATCH] branch: store the selection and expanded state We were currently saving the tree state as a sparse dict that can be used to restore the collapsed/expanded state of tree items. Extend this data structure to also include the selection state. The data structure currently stores the expanded state implicitly; existence of the dict means that the item is expanded. The expanded state will now store the expanded state alongside other data. Use separate dict entries for the expanded state, the selected state and the children dict. Update def _toggled_expanded() to only expand items and item when that item has children. This avoid additional refreshes of the UI because Qt will no longer re-layout the widgets below the clicked widget. The net result is that the selection state is restored across refreshes. No longer do we lose the user's selection in response to actions, and the UI no longer redraws itself as items are clicked. Closes #1221 Reported-by: Jakub Klos Signed-off-by: David Aguilar --- CHANGES.rst | 6 ++++++ cola/widgets/branch.py | 35 ++++++++++++++++++++++------------- test/branch_test.py | 29 ++++++++++++++++++++++++++--- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 752bfc70..eb4df41d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -3,6 +3,12 @@ v4.0.3 ====== +Usability, bells and whistles +----------------------------- +* The branches widget no longer loses its selection state in response to + notifications and UI actions. + (`#1221 `_) + Fixes ----- * The config reader has been revamped to better read settings when git config diff --git a/cola/widgets/branch.py b/cola/widgets/branch.py index 632aac94..ff1a1891 100644 --- a/cola/widgets/branch.py +++ b/cola/widgets/branch.py @@ -198,7 +198,9 @@ class BranchesTreeWidget(standard.TreeWidget): def _toggle_expanded(self, index): """Toggle expanded/collapsed state when items are clicked""" - self.setExpanded(index, not self.isExpanded(index)) + item = self.itemFromIndex(index) + if item and item.childCount(): + self.setExpanded(index, not self.isExpanded(index)) def contextMenuEvent(self, event): """Build and execute the context menu""" @@ -369,8 +371,7 @@ class BranchesTreeWidget(standard.TreeWidget): def _load_tree_state(self, states): """Restore expanded items after rebuilding UI widgets""" for item in self.items(): - if item.name in states: - self.tree_helper.load_state(item, states[item.name]) + self.tree_helper.load_state(item, states.get(item.name, {})) def _update_branches(self): """Query branch details using a background task""" @@ -701,22 +702,30 @@ def get_toplevel_item(item): class BranchesTreeHelper(object): def load_state(self, item, state): """Load expanded items from a dict""" - if state.keys(): + if state.get('expanded', False) and not item.isExpanded(): item.setExpanded(True) + if state.get('selected', False) and not item.isSelected(): + item.setSelected(True) + children_state = state.get('children', {}) for i in range(item.childCount()): child = item.child(i) - if child.name in state: - self.load_state(child, state[child.name]) + self.load_state(child, children_state.get(child.name, {})) def save_state(self, item): - """Save expanded items in a dict""" - result = {item.name: {}} - - if item.isExpanded(): - for i in range(item.childCount()): - child = item.child(i) - result[item.name].update(self.save_state(child)) + """Save the selected and expanded item state into a dict""" + expanded = item.isExpanded() + selected = item.isSelected() + children = {} + entry = { + 'children': children, + 'expanded': expanded, + 'selected': selected, + } + result = {item.name: entry} + for i in range(item.childCount()): + child = item.child(i) + children.update(self.save_state(child)) return result diff --git a/test/branch_test.py b/test/branch_test.py index 089f1e2e..1d167874 100644 --- a/test/branch_test.py +++ b/test/branch_test.py @@ -197,7 +197,7 @@ def test_should_return_empty_state_on_save_state(): top = _create_item('top', None, False) tree_helper = branch.BranchesTreeHelper() actual = tree_helper.save_state(top) - assert {'top': {}} == actual + assert {'top': {'children': {}, 'expanded': False, 'selected': False}} == actual def test_should_return_a_valid_state_on_save_state(): @@ -207,8 +207,31 @@ def test_should_return_a_valid_state_on_save_state(): actual = tree_helper.save_state(items['top']) expect = { 'top': { - 'child_1': {}, - 'child_2': {'sub_child_2_1': {}, 'sub_child_2_2': {}}, + 'children': { + 'child_1': { + 'children': {}, + 'expanded': False, + 'selected': False, + }, + 'child_2': { + 'children': { + 'sub_child_2_1': { + 'children': {}, + 'expanded': False, + 'selected': False, + }, + 'sub_child_2_2': { + 'children': {}, + 'expanded': False, + 'selected': False, + }, + }, + 'expanded': True, + 'selected': False, + }, + }, + 'expanded': True, + 'selected': False, } } assert expect == actual -- 2.11.4.GIT