Merge pull request #2463 from zas/itemviews_headers

Itemviews headers refactoring

picard/__init__.py

@@ -41,7 +41,7 @@
 PICARD_DISPLAY_NAME = "MusicBrainz Picard"
 PICARD_APP_ID = "org.musicbrainz.Picard"
 PICARD_DESKTOP_NAME = PICARD_APP_ID + ".desktop"
-PICARD_VERSION = Version(3, 0, 0, 'dev', 3)
+PICARD_VERSION = Version(3, 0, 0, 'dev', 4)
 
 
 # optional build version

picard/config_upgrade.py

@@ -542,6 +542,14 @@ def upgrade_to_v3_0_0dev3(config):
     rename_option(config, old_opt, new_opt, BoolOption, False)
 
 
+def upgrade_to_v3_0_0dev4(config):
+    """Reset "file/album_view_header_state" if there were saved while locked."""
+    if config.persist['album_view_header_locked']:
+        config.persist.remove('album_view_header_state')
+    if config.persist['file_view_header_locked']:
+        config.persist.remove('file_view_header_state')
+
+
 def rename_option(config, old_opt, new_opt, option_type, default):
     _s = config.setting
     if old_opt in _s:

picard/ui/itemviews.py

@@ -110,6 +110,8 @@
 ICON_SIZE = QtCore.QSize(COLUMN_ICON_SIZE+COLUMN_ICON_BORDER,
                          COLUMN_ICON_SIZE+COLUMN_ICON_BORDER)
 
+DEFAULT_SECTION_SIZE = 100
+
 
 def get_match_color(similarity, basecolor):
     c1 = (basecolor.red(), basecolor.green(), basecolor.blue())
@@ -327,7 +329,7 @@ def __init__(self, parent=None):
         # XXX it would be nice to be able to go to the 'no sort' mode, but the
         #     internal model that QTreeWidget uses doesn't support it
         self.setSortIndicator(-1, QtCore.Qt.SortOrder.AscendingOrder)
-        self.setDefaultSectionSize(100)
+        self.setDefaultSectionSize(DEFAULT_SECTION_SIZE)
 
     def show_column(self, column, show):
         if column == 0:  # The first column is fixed
@@ -396,24 +398,25 @@ def on_sort_indicator_changed(self, index, order):
 
     def lock(self, is_locked):
         super().lock(is_locked)
-        column_index = MainPanel.FINGERPRINT_COLUMN
-        if not self.is_locked and self.count() > column_index:
-            self.setSectionResizeMode(column_index, QtWidgets.QHeaderView.ResizeMode.Fixed)
+
+    def __str__(self):
+        name = getattr(self.parent(), 'NAME', str(self.parent().__class__.__name__))
+        return f"{name}'s header"
 
 
 class BaseTreeView(QtWidgets.QTreeWidget):
 
     def __init__(self, window, parent=None):
         super().__init__(parent)
+        self.setAccessibleName(_(self.NAME))
+        self.setAccessibleDescription(_(self.DESCRIPTION))
         self.tagger = QtCore.QCoreApplication.instance()
-        self.setHeader(ConfigurableColumnsHeader(self))
         self.window = window
         self.panel = parent
         # Should multiple files dropped be assigned to tracks sequentially?
         self._move_to_multi_tracks = True
-        self.setHeaderLabels([_(h) if n != '~fingerprint' else ''
-                              for h, n in MainPanel.columns])
-        self.restore_state()
+
+        self._init_header()
 
         self.setAcceptDrops(True)
         self.setDragEnabled(True)
@@ -649,28 +652,42 @@ def _add_other_versions():
     @restore_method
     def restore_state(self):
         config = get_config()
-        self._restore_state(config.persist[self.header_state])
-        self.header().lock(config.persist[self.header_locked])
+        self.restore_default_columns()
+
+        header_state = config.persist[self.header_state]
+        header = self.header()
+        if header_state and header.restoreState(header_state):
+            log.debug("Restoring state of %s" % header)
+            for i in range(0, self.columnCount()):
+                header.show_column(i, not self.isColumnHidden(i))
+
+        header.lock(config.persist[self.header_locked])
 
     def save_state(self):
         config = get_config()
-        config.persist[self.header_state] = self.header().saveState()
-        config.persist[self.header_locked] = self.header().is_locked
+        header = self.header()
+        if header.prelock_state is not None:
+            state = header.prelock_state
+        else:
+            state = header.saveState()
+        log.debug("Saving state of %s" % header)
+        config.persist[self.header_state] = state
+        config.persist[self.header_locked] = header.is_locked
 
     def restore_default_columns(self):
-        self._restore_state(None)
+        labels = [_(h) if n != '~fingerprint' else '' for h, n in MainPanel.columns]
+        self.setHeaderLabels(labels)
 
-    def _restore_state(self, header_state):
         header = self.header()
-        if header_state:
-            header.restoreState(header_state)
-            for i in range(0, self.columnCount()):
-                header.show_column(i, not self.isColumnHidden(i))
-        else:
-            header.update_visible_columns([0, 1, 2])
-            for i, size in enumerate([250, 50, 100]):
-                header.resizeSection(i, size)
-            self.sortByColumn(-1, QtCore.Qt.SortOrder.AscendingOrder)
+        header.update_visible_columns([0, 1, 2])
+        for i, size in enumerate([250, 50, 100]):
+            header.resizeSection(i, size)
+        self.sortByColumn(-1, QtCore.Qt.SortOrder.AscendingOrder)
+
+    def _init_header(self):
+        header = ConfigurableColumnsHeader(self)
+        self.setHeader(header)
+        self.restore_state()
 
     def supportedDropActions(self):
         return QtCore.Qt.DropAction.CopyAction | QtCore.Qt.DropAction.MoveAction
@@ -832,13 +849,14 @@ def default_drop_target(self):
 
 class FileTreeView(BaseTreeView):
 
+    NAME = N_("file view")
+    DESCRIPTION = N_("Contains unmatched files and clusters")
+
     header_state = 'file_view_header_state'
     header_locked = 'file_view_header_locked'
 
     def __init__(self, window, parent=None):
         super().__init__(window, parent)
-        self.setAccessibleName(_("file view"))
-        self.setAccessibleDescription(_("Contains unmatched files and clusters"))
         self.unmatched_files = ClusterItem(self.tagger.unclustered_files, False, self)
         self.unmatched_files.update()
         self.unmatched_files.setExpanded(True)
@@ -867,13 +885,14 @@ def default_drop_target(self):
 
 class AlbumTreeView(BaseTreeView):
 
+    NAME = N_("album view")
+    DESCRIPTION = N_("Contains albums and matched files")
+
     header_state = 'album_view_header_state'
     header_locked = 'album_view_header_locked'
 
     def __init__(self, window, parent=None):
         super().__init__(window, parent)
-        self.setAccessibleName(_("album view"))
-        self.setAccessibleDescription(_("Contains albums and matched files"))
         self.tagger.album_added.connect(self.add_album)
         self.tagger.album_removed.connect(self.remove_album)
 

picard/ui/widgets/tristatesortheaderview.py

@@ -40,6 +40,7 @@ class TristateSortHeaderView(QtWidgets.QHeaderView):
 
     def __init__(self, orientation, parent=None):
         super().__init__(orientation, parent)
+        self.prelock_state = None
 
         # Remember if resize / move event just happened
         self._section_moved_or_resized = False
@@ -83,10 +84,12 @@ def mouseReleaseEvent(self, event):
 
     def lock(self, is_locked):
         self.is_locked = is_locked
-        self.setSectionsClickable(not is_locked)
-        self.setSectionsMovable(not is_locked)
         if is_locked:
-            resize_mode = QtWidgets.QHeaderView.ResizeMode.Fixed
+            self.prelock_state = self.saveState()
+            self.setSectionResizeMode(QtWidgets.QHeaderView.ResizeMode.Fixed)
         else:
-            resize_mode = QtWidgets.QHeaderView.ResizeMode.Interactive
-        self.setSectionResizeMode(resize_mode)
+            if self.prelock_state is not None:
+                self.restoreState(self.prelock_state)
+                self.prelock_state = None
+        self.setSectionsClickable(not is_locked)
+        self.setSectionsMovable(not is_locked)

test/test_config_upgrade.py

@@ -65,6 +65,7 @@
     upgrade_to_v2_7_0dev5,
     upgrade_to_v2_8_0dev2,
     upgrade_to_v3_0_0dev3,
+    upgrade_to_v3_0_0dev4,
 )
 from picard.const.defaults import (
     DEFAULT_FILE_NAMING_FORMAT,
@@ -519,3 +520,24 @@ def test_upgrade_to_v3_0_0dev3(self):
         upgrade_to_v3_0_0dev3(self.config)
         self.assertNotIn('toolbar_multiselect', self.config.setting)
         self.assertTrue(self.config.setting['allow_multi_dirs_selection'])
+
+    def test_upgrade_to_v3_0_0dev4(self):
+        Option('persist', 'album_view_header_state', QByteArray())
+        Option('persist', 'file_view_header_state', QByteArray())
+        BoolOption('persist', 'album_view_header_locked', False)
+        BoolOption('persist', 'file_view_header_locked', False)
+
+        self.config.persist['album_view_header_state'] = b'foo'
+        self.config.persist['file_view_header_state'] = b'bar'
+
+        # test not locked, states shouldn't be modified
+        upgrade_to_v3_0_0dev4(self.config)
+        self.assertEqual(b'foo', self.config.persist['album_view_header_state'])
+        self.assertEqual(b'bar', self.config.persist['file_view_header_state'])
+
+        # test locked, states should be removed
+        self.config.persist['album_view_header_locked'] = True
+        self.config.persist['file_view_header_locked'] = True
+        upgrade_to_v3_0_0dev4(self.config)
+        self.assertEqual(b'', self.config.persist['album_view_header_state'])
+        self.assertEqual(b'', self.config.persist['file_view_header_state'])