From df1041767c1451f26e9e16fca240905338b2c584 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:34:08 +0200 Subject: [PATCH 1/7] ArtworkTable: pass parent to inherited class and class properties instead of hardcoded numbers for rows & cols --- picard/ui/infodialog.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index df1f9b941..94f01376a 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -93,8 +93,11 @@ class ArtworkCoverWidget(QtWidgets.QWidget): class ArtworkTable(QtWidgets.QTableWidget): - def __init__(self, display_existing_art): - super().__init__(0, 2) + NUM_ROWS = 0 + NUM_COLS = 2 + + def __init__(self, display_existing_art, parent=None): + super().__init__(self.NUM_ROWS, self.NUM_COLS, parent=parent) self.display_existing_art = display_existing_art h_header = self.horizontalHeader() v_header = self.verticalHeader() @@ -148,7 +151,7 @@ class InfoDialog(PicardDialog): self.ui.buttonBox.accepted.connect(self.accept) # Add the ArtworkTable to the ui - self.ui.artwork_table = ArtworkTable(self.display_existing_artwork) + self.ui.artwork_table = ArtworkTable(self.display_existing_artwork, parent=self) self.ui.artwork_table.setObjectName('artwork_table') self.ui.artwork_tab.layout().addWidget(self.ui.artwork_table) self.setTabOrder(self.ui.tabWidget, self.ui.artwork_table) From e75822e6f67bc05c72e86e70ebde073b26639457 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:35:57 +0200 Subject: [PATCH 2/7] ArtworkTable: use class properties instead of hardcoded numbers --- picard/ui/infodialog.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index 94f01376a..d3a61434b 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -93,16 +93,23 @@ class ArtworkCoverWidget(QtWidgets.QWidget): class ArtworkTable(QtWidgets.QTableWidget): + H_SIZE = 200 + V_SIZE = 230 + + TYPE_COLUMN_SIZE = 140 + NUM_ROWS = 0 NUM_COLS = 2 def __init__(self, display_existing_art, parent=None): super().__init__(self.NUM_ROWS, self.NUM_COLS, parent=parent) self.display_existing_art = display_existing_art + h_header = self.horizontalHeader() + h_header.setDefaultSectionSize(self.H_SIZE) v_header = self.verticalHeader() - h_header.setDefaultSectionSize(200) - v_header.setDefaultSectionSize(230) + v_header.setDefaultSectionSize(self.V_SIZE) + if self.display_existing_art: self._existing_cover_col = 0 self._type_col = 1 @@ -114,7 +121,7 @@ class ArtworkTable(QtWidgets.QTableWidget): self._type_col = 0 self._new_cover_col = 1 self.setHorizontalHeaderLabels([_("Type"), _("Cover")]) - self.setColumnWidth(self._type_col, 140) + self.setColumnWidth(self._type_col, self.TYPE_COLUMN_SIZE) class InfoDialog(PicardDialog): From 56de836fa552c4a312fe2b185b90e0b4043b0dd1 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:36:24 +0200 Subject: [PATCH 3/7] ArtworkTable: stretch last section --- picard/ui/infodialog.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index d3a61434b..c89066535 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -107,6 +107,8 @@ class ArtworkTable(QtWidgets.QTableWidget): h_header = self.horizontalHeader() h_header.setDefaultSectionSize(self.H_SIZE) + h_header.setStretchLastSection(True) + v_header = self.verticalHeader() v_header.setDefaultSectionSize(self.V_SIZE) From 2b09b9c46d2a3dc01020c278419dc58c95791962 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:37:13 +0200 Subject: [PATCH 4/7] ArtworkTable: shorten lines and reduce code redundancy --- picard/ui/infodialog.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index c89066535..6945e9147 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -117,13 +117,14 @@ class ArtworkTable(QtWidgets.QTableWidget): self._type_col = 1 self._new_cover_col = 2 self.insertColumn(2) - self.setHorizontalHeaderLabels([_("Existing Cover"), _("Type"), - _("New Cover")]) + labels = (_("Existing Cover"), _("Type"), _("New Cover"),) else: self._type_col = 0 self._new_cover_col = 1 - self.setHorizontalHeaderLabels([_("Type"), _("Cover")]) self.setColumnWidth(self._type_col, self.TYPE_COLUMN_SIZE) + labels = (_("Type"), _("Cover"),) + + self.setHorizontalHeaderLabels(labels) class InfoDialog(PicardDialog): From 12678e4b3762ecd7ecc3ab244708e4904e33e1c0 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:38:23 +0200 Subject: [PATCH 5/7] ArtworkTable: insert column at NUM_COLS position --- picard/ui/infodialog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index 6945e9147..c6bd73640 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -116,7 +116,7 @@ class ArtworkTable(QtWidgets.QTableWidget): self._existing_cover_col = 0 self._type_col = 1 self._new_cover_col = 2 - self.insertColumn(2) + self.insertColumn(self.NUM_COLS) labels = (_("Existing Cover"), _("Type"), _("New Cover"),) else: self._type_col = 0 From faf266bdde685b3156b7e011e7151783e21379ad Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 16:57:14 +0200 Subject: [PATCH 6/7] ArtworkTable: introduce get_column_index() method and drop access to private properties --- picard/ui/infodialog.py | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index c6bd73640..36596f9ae 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -113,19 +113,26 @@ class ArtworkTable(QtWidgets.QTableWidget): v_header.setDefaultSectionSize(self.V_SIZE) if self.display_existing_art: - self._existing_cover_col = 0 - self._type_col = 1 - self._new_cover_col = 2 + self._columns = { + 'existing_cover': 0, + 'type': 1, + 'new_cover': 2, + } self.insertColumn(self.NUM_COLS) labels = (_("Existing Cover"), _("Type"), _("New Cover"),) else: - self._type_col = 0 - self._new_cover_col = 1 - self.setColumnWidth(self._type_col, self.TYPE_COLUMN_SIZE) + self._columns = { + 'type': 0, + 'new_cover': 1, + } + self.setColumnWidth(self._columns['type'], self.TYPE_COLUMN_SIZE) labels = (_("Type"), _("Cover"),) self.setHorizontalHeaderLabels(labels) + def get_column_index(self, name): + return self._columns[name] + class InfoDialog(PicardDialog): @@ -189,19 +196,21 @@ class InfoDialog(PicardDialog): lambda s: '%s' % (color, text_as_html(s)), errors)) self.ui.error.setText(text + '
') - def _display_artwork(self, images, col): + def _display_artwork(self, images, cover_art_column_name): """Draw artwork in corresponding cell if image type matches type in Type column. Arguments: images -- The images to be drawn. - col -- Column in which images are to be drawn. Can be _new_cover_col or _existing_cover_col. + cover_art_column_name -- Column in which images are to be drawn. Can be 'new_cover' or 'existing_cover'. """ + artwork_col = self.artwork_table.get_column_index(cover_art_column_name) + type_col = self.artwork_table.get_column_index('type') row = 0 row_count = self.artwork_table.rowCount() missing_pixmap = QtGui.QPixmap(":/images/image-missing.png") for image in images: while row != row_count: - image_type = self.artwork_table.item(row, self.artwork_table._type_col) + image_type = self.artwork_table.item(row, type_col) if image_type and image_type.data(QtCore.Qt.ItemDataRole.UserRole) == image.types_as_string(): break row += 1 @@ -248,8 +257,8 @@ class InfoDialog(PicardDialog): infos.append(image.mimetype) img_wgt = ArtworkCoverWidget(pixmap=pixmap, text="\n".join(infos)) - self.artwork_table.setCellWidget(row, col, img_wgt) - self.artwork_table.setItem(row, col, item) + self.artwork_table.setCellWidget(row, artwork_col, img_wgt) + self.artwork_table.setItem(row, artwork_col, item) row += 1 def _display_artwork_type(self): @@ -264,21 +273,22 @@ class InfoDialog(PicardDialog): pixmap_arrow = QtGui.QPixmap(":/images/arrow.png") else: pixmap_arrow = None + type_col = self.artwork_table.get_column_index('type') for row, artwork_type in enumerate(types): self.artwork_table.insertRow(row) item = QtWidgets.QTableWidgetItem() item.setData(QtCore.Qt.ItemDataRole.UserRole, artwork_type) type_wgt = ArtworkCoverWidget(pixmap=pixmap_arrow, text=artwork_type) - self.artwork_table.setCellWidget(row, self.artwork_table._type_col, type_wgt) - self.artwork_table.setItem(row, self.artwork_table._type_col, item) + self.artwork_table.setCellWidget(row, type_col, type_wgt) + self.artwork_table.setItem(row, type_col, item) def _display_artwork_tab(self): if not self.images: self.tab_hide(self.ui.artwork_tab) self._display_artwork_type() - self._display_artwork(self.images, self.artwork_table._new_cover_col) + self._display_artwork(self.images, 'new_cover') if self.existing_images: - self._display_artwork(self.existing_images, self.artwork_table._existing_cover_col) + self._display_artwork(self.existing_images, 'existing_cover') self.artwork_table.itemDoubleClicked.connect(self.show_item) self.artwork_table.verticalHeader().resizeSections(QtWidgets.QHeaderView.ResizeMode.ResizeToContents) From c18febd534685cfd55db389aa3d98d4b5b2a6128 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 18 May 2024 17:27:42 +0200 Subject: [PATCH 7/7] Subclass ArtworkTable and drop display_existing_artwork - Make ArtworkTable more generic - Add 2 subclasses ArtworkTableSimple & ArtworkTableExisting --- picard/ui/infodialog.py | 63 ++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index 36596f9ae..20fa7eb8e 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -96,14 +96,14 @@ class ArtworkTable(QtWidgets.QTableWidget): H_SIZE = 200 V_SIZE = 230 - TYPE_COLUMN_SIZE = 140 - NUM_ROWS = 0 NUM_COLS = 2 - def __init__(self, display_existing_art, parent=None): + _columns = {} + _labels = () + + def __init__(self, parent=None): super().__init__(self.NUM_ROWS, self.NUM_COLS, parent=parent) - self.display_existing_art = display_existing_art h_header = self.horizontalHeader() h_header.setDefaultSectionSize(self.H_SIZE) @@ -112,28 +112,39 @@ class ArtworkTable(QtWidgets.QTableWidget): v_header = self.verticalHeader() v_header.setDefaultSectionSize(self.V_SIZE) - if self.display_existing_art: - self._columns = { - 'existing_cover': 0, - 'type': 1, - 'new_cover': 2, - } - self.insertColumn(self.NUM_COLS) - labels = (_("Existing Cover"), _("Type"), _("New Cover"),) - else: - self._columns = { - 'type': 0, - 'new_cover': 1, - } - self.setColumnWidth(self._columns['type'], self.TYPE_COLUMN_SIZE) - labels = (_("Type"), _("Cover"),) - - self.setHorizontalHeaderLabels(labels) + self.setHorizontalHeaderLabels(self._labels) def get_column_index(self, name): return self._columns[name] +class ArtworkTableSimple(ArtworkTable): + TYPE_COLUMN_SIZE = 140 + + _columns = { + 'type': 0, + 'new_cover': 1, + } + + _labels = (_("Type"), _("Cover"),) + + def __init__(self, parent=None): + super().__init__(parent=parent) + self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE) + + +class ArtworkTableExisting(ArtworkTable): + NUM_COLS = 3 + + _columns = { + 'existing_cover': 0, + 'type': 1, + 'new_cover': 2, + } + + _labels = (_("Existing Cover"), _("Type"), _("New Cover"),) + + class InfoDialog(PicardDialog): def __init__(self, obj, parent=None): @@ -142,7 +153,7 @@ class InfoDialog(PicardDialog): self.images = [] self.existing_images = [] self.ui = Ui_InfoDialog() - self.display_existing_artwork = False + artworktable_class = ArtworkTableSimple if (isinstance(obj, File) and isinstance(obj.parent, Track) @@ -153,7 +164,7 @@ class InfoDialog(PicardDialog): if (getattr(obj, 'orig_metadata', None) is not None and obj.orig_metadata.images and obj.orig_metadata.images != obj.metadata.images): - self.display_existing_artwork = True + artworktable_class = ArtworkTableExisting self.existing_images = obj.orig_metadata.images if obj.metadata.images: @@ -161,14 +172,14 @@ class InfoDialog(PicardDialog): if not self.images and self.existing_images: self.images = self.existing_images self.existing_images = [] - self.display_existing_artwork = False + artworktable_class = ArtworkTableSimple self.ui.setupUi(self) self.ui.buttonBox.addButton( StandardButton(StandardButton.CLOSE), QtWidgets.QDialogButtonBox.ButtonRole.AcceptRole) self.ui.buttonBox.accepted.connect(self.accept) # Add the ArtworkTable to the ui - self.ui.artwork_table = ArtworkTable(self.display_existing_artwork, parent=self) + self.ui.artwork_table = artworktable_class(parent=self) self.ui.artwork_table.setObjectName('artwork_table') self.ui.artwork_tab.layout().addWidget(self.ui.artwork_table) self.setTabOrder(self.ui.tabWidget, self.ui.artwork_table) @@ -266,7 +277,7 @@ class InfoDialog(PicardDialog): If both existing covers and new covers are to be displayed, take union of both cover types list. """ types = [image.types_as_string() for image in self.images] - if self.display_existing_artwork: + if isinstance(self.artwork_table, ArtworkTableExisting): existing_types = [image.types_as_string() for image in self.existing_images] # Merge both types and existing types list in sorted order. types = union_sorted_lists(types, existing_types)