Bound archive buttons enable/disable to context menu actions #1793
Conversation
| menu.addSeparator() | ||
| diff_action = QAction(self.bDiff.icon(), self.bDiff.text(), menu) | ||
| diff_action.triggered.connect(self.diff_action) | ||
| menu.addAction(diff_action) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Categorizing by impact would also be possible.
- Recalculate
- Diff
Those only extract data of vorta.
- mount
- extract
Those do not alter the repo but modify the filesystem
- rename
This is reversible.
- delete
This deletes data.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 👍
There was a problem hiding this comment.
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?
|
I have tested this locally and everything works as intended. This solves the issue of mismatched available actions in the context menu vs the archive buttons. The buttons are categorized by impact as discussed, and the separators have have been cleaned up. |
| 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()) |
Description
The context menu that appears when you right click on an archive has its own rules for when actions are enabled or disabled, separate from the rules for the buttons in the archive tab, even though they perform the same actions. I do not believe there is a reason for these buttons to ever be enabled in the context menu and disabled on the archive tab or vise versa.
In this PR, I have bound the "enabled" or "disabled" state of the actions in the archive right-click context menu to the state of their button equivalent in the archive tab. This will improve consistency, and prevent issues like the one I encountered here.
Related Issue
#1792
Motivation and Context
Improve consistency and prevent errors
How Has This Been Tested?
Tested locally, and passes Github test workflow
Types of changes
Checklist:
I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.