From c5faa125a68e327654f723a2e0be45d8e8ff6590 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:34:46 +0200 Subject: [PATCH 1/9] Use user_collections.values() as id_ isn't used, and drop numerical indexes --- picard/ui/collectionmenu.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index d7d25448f..7574f3ae4 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -52,9 +52,8 @@ class CollectionMenu(QtWidgets.QMenu): self._ignore_update = True self.clear() self.actions = [] - for id_, collection in sorted(user_collections.items(), - key=lambda k_v: - (strxfrm(str(k_v[1])), k_v[0])): + for collection in sorted(user_collections.values(), + key=lambda c: (strxfrm(c.name), c.id)): action = QtWidgets.QWidgetAction(self) action.setDefaultWidget(CollectionMenuItem(self, collection)) self.addAction(action) From 10521be80956b60d0c270170783e6721af39b946 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:37:25 +0200 Subject: [PATCH 2/9] CollectionMenu.__init__(): also pass keyword arguments to parent class --- picard/ui/collectionmenu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 7574f3ae4..8a9adf276 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -42,8 +42,8 @@ from picard.util import strxfrm class CollectionMenu(QtWidgets.QMenu): - def __init__(self, albums, *args): - super().__init__(*args) + def __init__(self, albums, *args, **kwargs): + super().__init__(*args, **kwargs) self.releases = set(a.id for a in albums) self._ignore_update = False self.update_collections() From 800a005ab428917763d95a5c113df340fa9ddf8c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:39:49 +0200 Subject: [PATCH 3/9] CollectionMenu.update_collections() -> _update_collections() --- picard/ui/collectionmenu.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 8a9adf276..1834499a3 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -46,9 +46,9 @@ class CollectionMenu(QtWidgets.QMenu): super().__init__(*args, **kwargs) self.releases = set(a.id for a in albums) self._ignore_update = False - self.update_collections() + self._update_collections() - def update_collections(self): + def _update_collections(self): self._ignore_update = True self.clear() self.actions = [] @@ -65,7 +65,7 @@ class CollectionMenu(QtWidgets.QMenu): def refresh_list(self): self.refresh_action.setEnabled(False) - load_user_collections(self.update_collections) + load_user_collections(self._update_collections) def mouseReleaseEvent(self, event): # Not using self.refresh_action.triggered because it closes the menu From 8f8cfa7f911869dad97f0939a88de9604e16ddcf Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:47:58 +0200 Subject: [PATCH 4/9] Always pass *args, **kwargs to parent Qt objects --- picard/ui/collectionmenu.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 1834499a3..3988d4021 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -93,8 +93,8 @@ class CollectionMenu(QtWidgets.QMenu): class CollectionMenuItem(QtWidgets.QWidget): - def __init__(self, menu, collection): - super().__init__() + def __init__(self, menu, collection, *args, **kwargs): + super().__init__(*args, **kwargs) self.menu = menu self.active = False self._setup_layout(menu, collection) @@ -108,7 +108,7 @@ class CollectionMenuItem(QtWidgets.QWidget): style.pixelMetric(QtWidgets.QStyle.PixelMetric.PM_FocusFrameVMargin), style.pixelMetric(QtWidgets.QStyle.PixelMetric.PM_LayoutRightMargin), style.pixelMetric(QtWidgets.QStyle.PixelMetric.PM_FocusFrameVMargin)) - self.checkbox = CollectionCheckBox(self, menu, collection) + self.checkbox = CollectionCheckBox(menu, collection, parent=self) layout.addWidget(self.checkbox) def _setup_colors(self): @@ -143,10 +143,10 @@ class CollectionMenuItem(QtWidgets.QWidget): class CollectionCheckBox(QtWidgets.QCheckBox): - def __init__(self, parent, menu, collection): + def __init__(self, menu, collection, *args, **kwargs): self.menu = menu self.collection = collection - super().__init__(self.label(), parent) + super().__init__(self.label(), *args, **kwargs) releases = collection.releases & menu.releases if len(releases) == len(menu.releases): From befe6a808e38d71209a1f8d7901882c10d36123e Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:51:57 +0200 Subject: [PATCH 5/9] refresh_list() -> _refresh_list() --- picard/ui/collectionmenu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 3988d4021..75a23c556 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -63,14 +63,14 @@ class CollectionMenu(QtWidgets.QMenu): self.refresh_action = self.addAction(_("Refresh List")) self.hovered.connect(self.update_highlight) - def refresh_list(self): + def _refresh_list(self): self.refresh_action.setEnabled(False) load_user_collections(self._update_collections) def mouseReleaseEvent(self, event): # Not using self.refresh_action.triggered because it closes the menu if self.actionAt(event.pos()) == self.refresh_action and self.refresh_action.isEnabled(): - self.refresh_list() + self._refresh_list() def update_highlight(self, action): if self._ignore_update: From 3b08914418f98ae11b2640ea0293df17c1d0d936 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:53:03 +0200 Subject: [PATCH 6/9] update_highlight() -> _on_hovered() --- picard/ui/collectionmenu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 75a23c556..4c7f28aee 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -61,7 +61,7 @@ class CollectionMenu(QtWidgets.QMenu): self._ignore_update = False self.addSeparator() self.refresh_action = self.addAction(_("Refresh List")) - self.hovered.connect(self.update_highlight) + self.hovered.connect(self._on_hovered) def _refresh_list(self): self.refresh_action.setEnabled(False) @@ -72,7 +72,7 @@ class CollectionMenu(QtWidgets.QMenu): if self.actionAt(event.pos()) == self.refresh_action and self.refresh_action.isEnabled(): self._refresh_list() - def update_highlight(self, action): + def _on_hovered(self, action): if self._ignore_update: return for a in self.actions: From 4fb4a66d8538194fac4dacff3a38f3d514c6b23a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 14:58:01 +0200 Subject: [PATCH 7/9] CollectionCheckBox: label() -> _label() --- picard/ui/collectionmenu.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 4c7f28aee..7336cfeea 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -146,7 +146,7 @@ class CollectionCheckBox(QtWidgets.QCheckBox): def __init__(self, menu, collection, *args, **kwargs): self.menu = menu self.collection = collection - super().__init__(self.label(), *args, **kwargs) + super().__init__(self._label(), *args, **kwargs) releases = collection.releases & menu.releases if len(releases) == len(menu.releases): @@ -169,9 +169,9 @@ class CollectionCheckBox(QtWidgets.QCheckBox): self.setCheckState(QtCore.Qt.CheckState.Unchecked) def updateText(self): - self.setText(self.label()) + self.setText(self._label()) - def label(self): + def _label(self): c = self.collection return ngettext("%(name)s (%(count)i release)", "%(name)s (%(count)i releases)", c.size) % { 'name': c.name, From 5f6e64c990e7701e11663c9df4e6620825b53269 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 15:02:55 +0200 Subject: [PATCH 8/9] CollectionCheckBox: updateText() -> _update_text() Camel-case was making the reader think it was an overloaded method, and that's not the case. --- picard/ui/collectionmenu.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 7336cfeea..3fc17e498 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -162,13 +162,13 @@ class CollectionCheckBox(QtWidgets.QCheckBox): return diff = releases - self.collection.releases if diff: - self.collection.add_releases(diff, self.updateText) + self.collection.add_releases(diff, self._update_text) self.setCheckState(QtCore.Qt.CheckState.Checked) else: - self.collection.remove_releases(releases & self.collection.releases, self.updateText) + self.collection.remove_releases(releases & self.collection.releases, self._update_text) self.setCheckState(QtCore.Qt.CheckState.Unchecked) - def updateText(self): + def _update_text(self): self.setText(self._label()) def _label(self): From 632f0805d8e3080734b612de14a90f9d057195a5 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Tue, 21 May 2024 15:12:50 +0200 Subject: [PATCH 9/9] Properly ignore hovered signal when set to do so `update_active_action_for_widget()` is using `self._ignore_hover` flag to disable handling of hovered signal, but this wasn't actually used, because, I think, of a typo in hovered signal handler which was testing `self._ignore_update` instead. Also `_ignore_hover` wasn't initialized. --- picard/ui/collectionmenu.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/picard/ui/collectionmenu.py b/picard/ui/collectionmenu.py index 3fc17e498..3317c5d71 100644 --- a/picard/ui/collectionmenu.py +++ b/picard/ui/collectionmenu.py @@ -46,6 +46,7 @@ class CollectionMenu(QtWidgets.QMenu): super().__init__(*args, **kwargs) self.releases = set(a.id for a in albums) self._ignore_update = False + self._ignore_hover = False self._update_collections() def _update_collections(self): @@ -73,7 +74,7 @@ class CollectionMenu(QtWidgets.QMenu): self._refresh_list() def _on_hovered(self, action): - if self._ignore_update: + if self._ignore_hover: return for a in self.actions: a.defaultWidget().set_active(a == action)