-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add batch actions top bar element #2233
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -55,6 +55,7 @@ | |||||||||||||||||||||||||||
QSizePolicy, | ||||||||||||||||||||||||||||
QSpacerItem, | ||||||||||||||||||||||||||||
QStatusBar, | ||||||||||||||||||||||||||||
QToolBar, | ||||||||||||||||||||||||||||
QToolButton, | ||||||||||||||||||||||||||||
QVBoxLayout, | ||||||||||||||||||||||||||||
QWidget, | ||||||||||||||||||||||||||||
|
@@ -215,14 +216,16 @@ def __init__(self) -> None: | |||||||||||||||||||||||||||
layout.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
layout.setSpacing(0) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self.user_profile = UserProfile() | ||||||||||||||||||||||||||||
self.branding_barre = QLabel() | ||||||||||||||||||||||||||||
self.branding_barre.setPixmap(load_image("left_pane.svg")) | ||||||||||||||||||||||||||||
self.user_profile = UserProfile() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Hide user profile widget until user logs in | ||||||||||||||||||||||||||||
self.user_profile.hide() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Add widgets to layout | ||||||||||||||||||||||||||||
# Add widgets to layout. An improvement | ||||||||||||||||||||||||||||
# to this layout could be to set the branding barre as a | ||||||||||||||||||||||||||||
# background layout for the other elements | ||||||||||||||||||||||||||||
layout.addWidget(self.user_profile) | ||||||||||||||||||||||||||||
layout.addWidget(self.branding_barre) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
@@ -424,6 +427,113 @@ def clear_message(self) -> None: | |||||||||||||||||||||||||||
self._hide() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class InnerTopPane(QWidget): | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
Top pane of the MainView window. This pane holds the Batch Action layout, | ||||||||||||||||||||||||||||
and eventually will hold the keyword search/filter by codename bar. | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def __init__(self) -> None: | ||||||||||||||||||||||||||||
super().__init__() | ||||||||||||||||||||||||||||
self.setObjectName("InnerTopPane") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Use a vertical layout so that the keyword search bar can be added later | ||||||||||||||||||||||||||||
layout = QVBoxLayout(self) | ||||||||||||||||||||||||||||
layout.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
layout.setSpacing(0) | ||||||||||||||||||||||||||||
layout.setAlignment(Qt.AlignVCenter) | ||||||||||||||||||||||||||||
self.setLayout(layout) | ||||||||||||||||||||||||||||
self.setAttribute(Qt.WA_StyledBackground, True) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self.batch_actions = BatchActionWidget() | ||||||||||||||||||||||||||||
layout.addWidget(self.batch_actions) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def setup(self, controller: Controller) -> None: | ||||||||||||||||||||||||||||
self.batch_actions.setup(controller) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class BatchActionWidget(QWidget): | ||||||||||||||||||||||||||||
def __init__(self) -> None: | ||||||||||||||||||||||||||||
super().__init__() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# CSS style id | ||||||||||||||||||||||||||||
self.setObjectName("BatchActionWidget") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Solid background colour | ||||||||||||||||||||||||||||
self.setAttribute(Qt.WA_StyledBackground, True) | ||||||||||||||||||||||||||||
layout = QHBoxLayout() | ||||||||||||||||||||||||||||
self.setLayout(layout) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self.toolbar = BatchActionToolbar() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
layout.addWidget(self.toolbar) | ||||||||||||||||||||||||||||
layout.addStretch() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def setup(self, controller: Controller) -> None: | ||||||||||||||||||||||||||||
self.toolbar.setup(controller) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class BatchActionToolbar(QToolBar): | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
A toolbar that contains batch actions (actions that target multiple | ||||||||||||||||||||||||||||
sources in the ConversationView, and therefore don't belong in the | ||||||||||||||||||||||||||||
individual conversation menu). Currently, this widget will hold the | ||||||||||||||||||||||||||||
"Delete Sources" (batch-delete) action. | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
For user-facing naming consistency, these items won't be called | ||||||||||||||||||||||||||||
"batch/bulk <delete>", but simply "<verb> <noun>s" (eg "Delete Sources"), where | ||||||||||||||||||||||||||||
the original nomenclature comes from the individual Source overflow QAction menu | ||||||||||||||||||||||||||||
items. Each item may have a tooltip, visible on hover, that provides a more | ||||||||||||||||||||||||||||
lengthy explanation (e.g., "Delete multiple source accounts"). | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def __init__(self) -> None: | ||||||||||||||||||||||||||||
super().__init__() | ||||||||||||||||||||||||||||
self.setObjectName("BatchActionToolbar") | ||||||||||||||||||||||||||||
self.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
palette = QPalette() | ||||||||||||||||||||||||||||
palette.setBrush( | ||||||||||||||||||||||||||||
QPalette.Background, QBrush(Qt.NoBrush) | ||||||||||||||||||||||||||||
) # This makes the widget transparent | ||||||||||||||||||||||||||||
self.setPalette(palette) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Style and attributes | ||||||||||||||||||||||||||||
self.setMovable(False) | ||||||||||||||||||||||||||||
self.setToolButtonStyle(Qt.ToolButtonStyle.ToolButtonTextBesideIcon) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# TODO: Icon needs changing? | ||||||||||||||||||||||||||||
# TODO: set keyboard shortcut | ||||||||||||||||||||||||||||
batch_delete_action = QAction( | ||||||||||||||||||||||||||||
QIcon(load_image("delete_close.svg")), _("DELETE SOURCES"), self | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
batch_delete_action.setObjectName("BatchActionButton") | ||||||||||||||||||||||||||||
batch_delete_action.setToolTip(_("Delete selected source accounts")) | ||||||||||||||||||||||||||||
batch_delete_action.triggered.connect(self.delete_multiple_sources) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# TODO: Enhancement: start with action disabled via setEnabled(False), | ||||||||||||||||||||||||||||
# enable when sources are selected | ||||||||||||||||||||||||||||
self.addAction(batch_delete_action) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def delete_multiple_sources(self) -> None: | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
Requires logged-in session. Delete currently-selected sources. | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
logger.debug("delete_multiple_sources triggered") | ||||||||||||||||||||||||||||
if self.controller.api is None: | ||||||||||||||||||||||||||||
self.controller.on_action_requiring_login() | ||||||||||||||||||||||||||||
Comment on lines
+518
to
+524
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can eliminate the conditional with:
Suggested change
|
||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||
# TODO rm: this is a placeholder | ||||||||||||||||||||||||||||
logger.debug("Delete sources clicked") | ||||||||||||||||||||||||||||
# TODO in followup PR | ||||||||||||||||||||||||||||
# An in-memory set of Sources selected by the user to be queued for deletion will | ||||||||||||||||||||||||||||
# live in the main app. If logged in, pass those to delete confirmation dialog, | ||||||||||||||||||||||||||||
# and if the user accepts the dialog, the sources will be deleted. | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def setup(self, controller: Controller) -> None: | ||||||||||||||||||||||||||||
self.controller = controller | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class UserProfile(QLabel): | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
A widget that contains user profile information and options. | ||||||||||||||||||||||||||||
|
@@ -603,8 +713,8 @@ def _on_clicked(self) -> None: | |||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class MainView(QWidget): | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
Represents the main content of the application (containing the source list | ||||||||||||||||||||||||||||
and main context view). | ||||||||||||||||||||||||||||
Represents the main content of the application (containing the source list, | ||||||||||||||||||||||||||||
main context view, and top actions pane). | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def __init__( | ||||||||||||||||||||||||||||
|
@@ -620,12 +730,20 @@ def __init__( | |||||||||||||||||||||||||||
self.setObjectName("MainView") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Set layout | ||||||||||||||||||||||||||||
self._layout = QHBoxLayout(self) | ||||||||||||||||||||||||||||
self._layout = QVBoxLayout(self) | ||||||||||||||||||||||||||||
self._layout.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
self._layout.setSpacing(0) | ||||||||||||||||||||||||||||
self.setLayout(self._layout) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Top pane layout (actions/eventual searchbar) | ||||||||||||||||||||||||||||
self.top_pane = InnerTopPane() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Hold main conversation view and sourcelist | ||||||||||||||||||||||||||||
inner_container = QHBoxLayout(self) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Set margins and spacing | ||||||||||||||||||||||||||||
self._layout.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
self._layout.setSpacing(0) | ||||||||||||||||||||||||||||
inner_container.setContentsMargins(0, 0, 0, 0) | ||||||||||||||||||||||||||||
inner_container.setSpacing(0) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Create SourceList widget | ||||||||||||||||||||||||||||
self.source_list = SourceList() | ||||||||||||||||||||||||||||
|
@@ -647,8 +765,11 @@ def __init__( | |||||||||||||||||||||||||||
self.view_layout.addWidget(self.empty_conversation_view) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Add widgets to layout | ||||||||||||||||||||||||||||
self._layout.addWidget(self.source_list, stretch=1) | ||||||||||||||||||||||||||||
self._layout.addWidget(self.view_holder, stretch=2) | ||||||||||||||||||||||||||||
inner_container.addWidget(self.source_list, stretch=1) | ||||||||||||||||||||||||||||
inner_container.addWidget(self.view_holder, stretch=2) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self._layout.addWidget(self.top_pane) | ||||||||||||||||||||||||||||
self._layout.addLayout(inner_container, stretch=1) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Note: We should not delete SourceConversationWrapper when its source is unselected. This | ||||||||||||||||||||||||||||
# is a temporary solution to keep copies of our objects since we do delete them. | ||||||||||||||||||||||||||||
|
@@ -660,6 +781,7 @@ def setup(self, controller: Controller) -> None: | |||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
self.controller = controller | ||||||||||||||||||||||||||||
self.source_list.setup(controller) | ||||||||||||||||||||||||||||
self.top_pane.setup(controller) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def show_sources(self, sources: list[Source]) -> None: | ||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the local style guide for "here's how to use this section of the UI". I'll note for the record that I continue to think that "batch" is an implementation detail of what is really just a "bulk" operation or an operation over multiple objects—not all of which will batch the individual operations or requests. I'm fine with whatever terminology we adopt as long as we don't wind up confused by it.