Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bound archive buttons enable/disable to context menu actions #1793

Merged
merged 5 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 51 additions & 51 deletions src/vorta/assets/UI/archivetab.ui
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,25 @@
</property>
</widget>
</item>
<item>
<widget class="QToolButton" name="bDiff">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="toolTip">
<string>Compare two archives</string>
</property>
<property name="text">
<string>Diff</string>
</property>
<property name="toolButtonStyle">
<enum>Qt::ToolButtonTextBesideIcon</enum>
</property>
</widget>
</item>
<item>
<widget class="QToolButton" name="bMountArchive">
<property name="sizePolicy">
Expand Down Expand Up @@ -276,60 +295,41 @@
</property>
</widget>
</item>
<item>
<widget class="QToolButton" name="bDelete">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="toolTip">
<string>Delete selected archive(s)</string>
</property>
<property name="text">
<string>Delete</string>
</property>
<property name="toolButtonStyle">
<enum>Qt::ToolButtonTextBesideIcon</enum>
</property>
</widget>
</item>
<item>
<spacer name="verticalSpacer">
<property name="orientation">
<enum>Qt::Vertical</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>20</width>
<height>40</height>
</size>
</property>
</spacer>
</item>
</layout>
</widget>
</item>
<item>
<spacer name="verticalSpacer_2">
<property name="orientation">
<enum>Qt::Vertical</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>20</width>
<height>40</height>
</size>
</property>
</spacer>
</item>
<item>
<widget class="QToolButton" name="bDiff">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="toolTip">
<string>Compare two archives</string>
</property>
<property name="text">
<string>Diff</string>
</property>
<property name="toolButtonStyle">
<enum>Qt::ToolButtonTextBesideIcon</enum>
</property>
</widget>
</item>
<item>
<widget class="QToolButton" name="bDelete">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="toolTip">
<string>Delete selected archive(s)</string>
</property>
<property name="text">
<string>Delete</string>
</property>
<property name="toolButtonStyle">
<enum>Qt::ToolButtonTextBesideIcon</enum>
</property>
</widget>
</item>
</layout>
</item>
</layout>
Expand Down
57 changes: 17 additions & 40 deletions src/vorta/views/archive_tab.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from PyQt6 import QtCore, uic
from PyQt6.QtCore import QItemSelectionModel, QMimeData, QPoint, Qt, pyqtSlot
from PyQt6.QtGui import QAction, QDesktopServices, QKeySequence, QShortcut
from PyQt6.QtGui import QDesktopServices, QKeySequence, QShortcut
from PyQt6.QtWidgets import (
QAbstractItemView,
QApplication,
Expand Down Expand Up @@ -154,7 +154,7 @@ def __init__(self, parent=None, app=None):
self.app.paletteChanged.connect(lambda p: self.set_icons())

def set_icons(self):
"Used when changing between light- and dark mode"
"""Used when changing between light- and dark mode"""
self.bCheck.setIcon(get_colored_icon('check-circle'))
self.bDiff.setIcon(get_colored_icon('stream-solid'))
self.bPrune.setIcon(get_colored_icon('cut'))
Expand Down Expand Up @@ -183,46 +183,22 @@ def archiveitem_contextmenu(self, pos: QPoint):
return # popup only for selected items

menu = QMenu(self.archiveTable)
menu.addAction(
get_colored_icon('copy'),
self.tr("Copy"),
lambda: self.archive_copy(index=index),
)
menu.addAction(get_colored_icon('copy'), self.tr("Copy"), lambda: self.archive_copy(index=index))
menu.addSeparator()

# archive actions
archive_actions = []
archive_actions.append(
menu.addAction(
self.bRefreshArchive.icon(),
self.bRefreshArchive.text(),
self.refresh_archive_info,
)
)
archive_actions.append(
menu.addAction(
self.bMountArchive.icon(),
self.bMountArchive.text(),
self.bmountarchive_clicked,
)
)
archive_actions.append(menu.addAction(self.bExtract.icon(), self.bExtract.text(), self.extract_action))
archive_actions.append(menu.addAction(self.bRename.icon(), self.bRename.text(), self.cell_double_clicked))
# deletion possible with one but also multiple archives
menu.addAction(self.bDelete.icon(), self.bDelete.text(), self.delete_action)

if not (self.repoactions_enabled and len(selected_rows) <= 1):
for action in archive_actions:
action.setEnabled(False)

# diff action
menu.addSeparator()
diff_action = QAction(self.bDiff.icon(), self.bDiff.text(), menu)
diff_action.triggered.connect(self.diff_action)
menu.addAction(diff_action)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, ideally, we would have the action menu mimic the layout of the button menu, with a separator before 'diff' and moving 'delete' to the end?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make sense although we should review the current button layout. I initialls designed it so that it distinguishes between single and multi archive options. But maybe there is a better button order.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially designed it so that it distinguishes between single and multi archive options

Let's run with that idea. Maybe list them by number of archives required for the action?

  • Single archive only actions: 'mount', 'extract', 'rename'
  • two archive only actions: 'diff'
  • any number archive actions: 'recalculate', 'delete'

Separators between each of these sections, maybe?

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Categorizing by impact would also be possible.

  1. Recalculate
  2. Diff

Those only extract data of vorta.

  1. mount
  2. extract

Those do not alter the repo but modify the filesystem

  1. rename

This is reversible.

  1. delete

This deletes data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can wrap these changes into this PR. Would we like separators between those categories, or just ordered in a list as you outlined?

Same list order for the context menu as well?

Copy link
Collaborator

@real-yfprojects real-yfprojects Sep 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which of these two options do you prefer?

Would we like separators between those categories, or just ordered in a list as you outlined?

I feel like separators or increased spacing would clutter the ui too much.

Same list order for the context menu as well?

Recognisable patterns repeated throughout the application usually improve the UX.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which of these two options do you prefer?

Categorizing by impact as you suggest makes logical sense, and I like 'delete' being the last option as that is intuitive.

I will remove the serparators and match the ordering for both button and context menu 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, the requested changes have been made. The only difference now between the context menu and the buttons is the 'copy' context menu action. Would you like there to be a 'copy' button as well, or leave it as it is?


selected_rows = self.archiveTable.selectionModel().selectedRows(index.column())
diff_action.setEnabled(self.repoactions_enabled and len(selected_rows) == 2)
button_connection_pairs = [
(self.bRefreshArchive, self.refresh_archive_info),
(self.bDiff, self.diff_action),
(self.bMountArchive, self.bmountarchive_clicked),
(self.bExtract, self.extract_action),
(self.bRename, self.cell_double_clicked),
(self.bDelete, self.delete_action),
]

for button, connection in button_connection_pairs:
action = menu.addAction(button.icon(), button.text(), connection)
action.setEnabled(button.isEnabled())
Comment on lines +190 to +201
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever rewrite!


menu.popup(self.archiveTable.viewport().mapToGlobal(pos))

Expand Down Expand Up @@ -406,7 +382,8 @@ def on_selection_change(self, selected=None, deselected=None):

for index in range(layout.count()):
widget = layout.itemAt(index).widget()
widget.setToolTip(self.tooltip_dict.get(widget, ""))
if widget is not None:
widget.setToolTip(self.tooltip_dict.get(widget, ""))

# refresh bMountArchive for the selected archive
self.bmountarchive_refresh()
Expand Down