From a3b267738e48b2b284df2bee7a7ff86d322eba1d Mon Sep 17 00:00:00 2001 From: John Hensley Date: Fri, 16 Jul 2021 17:04:11 -0400 Subject: [PATCH] Address review feedback --- securedrop_client/gui/widgets.py | 220 ++++++++++-------- .../resources/css/modal_dialog_button.css | 4 +- securedrop_client/resources/css/sdclient.css | 23 +- .../resources/css/source_menu.css | 10 +- tests/gui/test_widgets.py | 25 +- tests/integration/test_styles_sdclient.py | 28 +-- 6 files changed, 183 insertions(+), 127 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index a7361a51d3..782cdb1cf8 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -1175,8 +1175,8 @@ def __init__(self): layout.setSpacing(0) layout.addWidget(deletion_message, 0, 0, Qt.AlignRight | Qt.AlignVCenter) layout.addWidget(spinner, 0, 1, Qt.AlignLeft | Qt.AlignVCenter) - layout.setColumnStretch(0, 2) - layout.setColumnStretch(1, 1) + layout.setColumnStretch(0, 9) + layout.setColumnStretch(1, 7) self.setLayout(layout) @@ -1222,8 +1222,8 @@ def __init__(self): layout.addWidget(self.deletion_message, 0, 0, Qt.AlignRight | Qt.AlignVCenter) layout.addWidget(spinner, 0, 1, Qt.AlignLeft | Qt.AlignVCenter) - layout.setColumnStretch(0, 2) - layout.setColumnStretch(1, 1) + layout.setColumnStretch(0, 9) + layout.setColumnStretch(1, 7) self.setLayout(layout) @@ -2714,38 +2714,17 @@ class ModalDialog(QDialog): MARGIN = 40 NO_MARGIN = 0 - def __init__(self, show_header=True, dangerous=False): + def __init__(self, show_header: bool = True, dangerous: bool = False): parent = QApplication.activeWindow() super().__init__(parent) self.setObjectName("ModalDialog") self.setModal(True) + self.show_header = show_header self.dangerous = dangerous if self.dangerous: self.setProperty("class", "dangerous") - # Header for icon and task title - header_container = QWidget() - header_container_layout = QHBoxLayout() - header_container.setLayout(header_container_layout) - self.header_icon = SvgLabel("blank.svg", svg_size=QSize(64, 64)) - self.header_icon.setObjectName("ModalDialog_header_icon") - self.header_spinner = QPixmap() - self.header_spinner_label = QLabel() - self.header_spinner_label.setObjectName("ModalDialog_header_spinner") - self.header_spinner_label.setMinimumSize(64, 64) - self.header_spinner_label.setVisible(False) - self.header_spinner_label.setPixmap(self.header_spinner) - self.header = QLabel() - self.header.setObjectName("ModalDialog_header") - header_container_layout.addWidget(self.header_icon) - header_container_layout.addWidget(self.header_spinner_label) - header_container_layout.addWidget(self.header, alignment=Qt.AlignCenter) - header_container_layout.addStretch() - - self.header_line = QWidget() - self.header_line.setObjectName("ModalDialog_header_line") - # Widget for displaying error messages self.error_details = QLabel() self.error_details.setObjectName("ModalDialog_error_details") @@ -2760,43 +2739,46 @@ def __init__(self, show_header=True, dangerous=False): self.body.setScaledContents(True) body_container = QWidget() self.body_layout = QVBoxLayout() - self.body_layout.setContentsMargins(self.MARGIN, self.NO_MARGIN, self.MARGIN, self.MARGIN) + self.body_layout.setContentsMargins( + self.NO_MARGIN, self.NO_MARGIN, self.NO_MARGIN, self.NO_MARGIN + ) body_container.setLayout(self.body_layout) self.body_layout.addWidget(self.body) - # Buttons to continue and cancel - window_buttons = QWidget() - window_buttons.setObjectName("ModalDialog_window_buttons") - button_layout = QVBoxLayout() - window_buttons.setLayout(button_layout) - self.cancel_button = QPushButton(_("CANCEL")) - self.cancel_button.setObjectName("ModalDialog_cancel_button") - self.cancel_button.setStyleSheet(self.BUTTON_CSS) - self.cancel_button.clicked.connect(self.close) - self.cancel_button.setAutoDefault(False) - self.continue_button = QPushButton(_("CONTINUE")) - self.continue_button.setObjectName("ModalDialog_primary_button") - self.continue_button.setStyleSheet(self.BUTTON_CSS) - self.continue_button.setDefault(True) - self.continue_button.setIconSize(QSize(21, 21)) - button_box = QDialogButtonBox(Qt.Horizontal) - button_box.setObjectName("ModalDialog_button_box") - button_box.addButton(self.cancel_button, QDialogButtonBox.ActionRole) - button_box.addButton(self.continue_button, QDialogButtonBox.ActionRole) - button_layout.addWidget(button_box, alignment=Qt.AlignRight) - button_layout.setContentsMargins(self.NO_MARGIN, self.NO_MARGIN, self.MARGIN, self.MARGIN) - # Main widget layout layout = QVBoxLayout(self) - layout.setContentsMargins(40, 40, 40, 40) + layout.setContentsMargins(self.MARGIN, self.MARGIN, self.MARGIN, self.MARGIN) self.setLayout(layout) + if self.show_header: + # Header for icon and task title + header_container = QWidget() + header_container_layout = QHBoxLayout() + header_container.setLayout(header_container_layout) + self.header_icon = SvgLabel("blank.svg", svg_size=QSize(64, 64)) + self.header_icon.setObjectName("ModalDialog_header_icon") + self.header_spinner = QPixmap() + self.header_spinner_label = QLabel() + self.header_spinner_label.setObjectName("ModalDialog_header_spinner") + self.header_spinner_label.setMinimumSize(64, 64) + self.header_spinner_label.setVisible(False) + self.header_spinner_label.setPixmap(self.header_spinner) + self.header = QLabel() + self.header.setObjectName("ModalDialog_header") + header_container_layout.addWidget(self.header_icon) + header_container_layout.addWidget(self.header_spinner_label) + header_container_layout.addWidget(self.header, alignment=Qt.AlignCenter) + header_container_layout.addStretch() + + self.header_line = QWidget() + self.header_line.setObjectName("ModalDialog_header_line") + layout.addWidget(header_container) layout.addWidget(self.header_line) + layout.addWidget(self.error_details) layout.addWidget(body_container) - layout.addStretch() - layout.addWidget(window_buttons) + layout.addWidget(self.configure_buttons()) # Activestate animation. self.button_animation = load_movie("activestate-wide.gif") @@ -2808,6 +2790,51 @@ def __init__(self, show_header=True, dangerous=False): self.header_animation.setScaledSize(QSize(64, 64)) self.header_animation.frameChanged.connect(self.animate_header) + def configure_buttons(self): + # Buttons to continue and cancel + window_buttons = QWidget() + window_buttons.setObjectName("ModalDialog_window_buttons") + + button_layout = QVBoxLayout() + window_buttons.setLayout(button_layout) + + self.cancel_button = QPushButton(_("CANCEL")) + self.cancel_button.setStyleSheet(self.BUTTON_CSS) + self.cancel_button.clicked.connect(self.close) + + self.continue_button = QPushButton(_("CONTINUE")) + self.continue_button.setStyleSheet(self.BUTTON_CSS) + self.continue_button.setIconSize(QSize(21, 21)) + + button_box = QDialogButtonBox(Qt.Horizontal) + button_box.setObjectName("ModalDialog_button_box") + + if self.dangerous: + self.cancel_button.setAutoDefault(True) + self.continue_button.setDefault(False) + self.cancel_button.setObjectName("ModalDialog_primary_button") + self.continue_button.setObjectName("ModalDialog_cancel_button") + else: + self.cancel_button.setAutoDefault(False) + self.continue_button.setDefault(True) + self.cancel_button.setObjectName("ModalDialog_cancel_button") + self.continue_button.setObjectName("ModalDialog_primary_button") + + button_box.addButton(self.cancel_button, QDialogButtonBox.ActionRole) + button_box.addButton(self.continue_button, QDialogButtonBox.ActionRole) + + self.confirmation_label = QLabel() + self.confirmation_label.setObjectName("ModalDialogConfirmation") + button_layout.addWidget(self.confirmation_label, 0, Qt.AlignLeft | Qt.AlignBottom) + + button_layout.addWidget(button_box, alignment=Qt.AlignLeft) + + button_layout.setContentsMargins( + self.NO_MARGIN, self.NO_MARGIN, self.NO_MARGIN, self.NO_MARGIN + ) + + return window_buttons + def keyPressEvent(self, event: QKeyEvent): if event.key() == Qt.Key_Enter or event.key() == Qt.Key_Return: if self.cancel_button.hasFocus(): @@ -3245,31 +3272,32 @@ def __init__(self, source, controller): self.controller = controller self.body.setText(self.make_body_text()) - self.adjustSize() - self.continue_button.setText(_("YES, DELETE SOURCE ACCOUNT")) + self.continue_button.setText(_("YES, DELETE ENTIRE SOURCE ACCOUNT")) self.continue_button.clicked.connect(self.delete_source) + self.confirmation_label.setText(_("Are you sure this is what you want?")) + + self.adjustSize() + def make_body_text(self) -> str: message_tuple = ( - "", + "", "

", _("When the entire account for a source is deleted:"), "

", - "", - "

", - _("Are you sure this is what you want for {source}?"), "

", + "

 

", ) return "".join(message_tuple).format( @@ -3294,11 +3322,12 @@ def __init__(self, source, controller): self.controller = controller self.body.setText(self.make_body_text()) - self.adjustSize() self.continue_button.setText(_("YES, DELETE FILES AND MESSAGES")) self.continue_button.clicked.connect(self.delete_conversation) + self.adjustSize() + def make_body_text(self) -> str: files = 0 messages = 0 @@ -3314,34 +3343,36 @@ def make_body_text(self) -> str: message_tuple = ( "", "

", - _("This will delete the following content associated with source {source}:"), - "

", - "", + "

", "

", _( - "This will not delete the account or its metadata, and the source {source} " - "will still be able to log in to your SecureDrop and communicate with you." + "Preserving the account will retain its metadata, and the ability for {source} " + "to log in to your SecureDrop again." ), "

", ) + files_to_delete = ngettext("one file", "{file_count} files", files).format(file_count=files) + + replies_to_delete = ngettext("one reply", "{reply_count} replies", replies).format( + reply_count=replies + ) + + messages_to_delete = ngettext("one message", "{message_count} messages", messages).format( + message_count=messages + ) + + source = "{}".format(self.source.journalist_designation) + return "".join(message_tuple).format( - files=files, - messages=messages, - replies=replies, - source="{}".format(self.source.journalist_designation), + files_to_delete=files_to_delete, + messages_to_delete=messages_to_delete, + replies_to_delete=replies_to_delete, + source=source, ) @pyqtSlot() @@ -4011,14 +4042,6 @@ def update_label_width(self, width): self.source_name_label.setText(self.source_name) -class DeleteSeparatorAction(QAction): - def __init__(self, parent): - self.text = _("DELETE") - - super().__init__(self.text, parent) - self.setSeparator(True) - - class DeleteSourceAction(QAction): """Use this action to delete the source record.""" @@ -4079,10 +4102,11 @@ def __init__(self, source, controller): self.setStyleSheet(self.SOURCE_MENU_CSS) separator_font = QFont() separator_font.setLetterSpacing(QFont.AbsoluteSpacing, 2) + separator_font.setBold(True) - delete_section = DeleteSeparatorAction(self) + delete_section = self.addSection(_("DELETE")) delete_section.setFont(separator_font) - self.addAction(delete_section) + self.addAction(DeleteConversationAction(self.source, self, self.controller)) self.addAction(DeleteSourceAction(self.source, self, self.controller)) diff --git a/securedrop_client/resources/css/modal_dialog_button.css b/securedrop_client/resources/css/modal_dialog_button.css index 6cee1c7d68..132952a4bd 100644 --- a/securedrop_client/resources/css/modal_dialog_button.css +++ b/securedrop_client/resources/css/modal_dialog_button.css @@ -24,8 +24,6 @@ background-color: #f1f1f6; color: #fff; border: 2px solid #f1f1f6; - margin: 0px 0px 0px 12px; + margin: 0; height: 40px; - padding-left: 20px; - padding-right: 20px; } diff --git a/securedrop_client/resources/css/sdclient.css b/securedrop_client/resources/css/sdclient.css index e17b46cf78..e9577acb36 100644 --- a/securedrop_client/resources/css/sdclient.css +++ b/securedrop_client/resources/css/sdclient.css @@ -437,7 +437,7 @@ QWidget#FileWidget_horizontal_line { #ModalDialog_header { min-height: 68px; max-height: 68px; - margin: 0px 0px 0px 4px; + margin: 0; font-family: 'Montserrat'; font-size: 24px; font-weight: 600; @@ -445,7 +445,7 @@ QWidget#FileWidget_horizontal_line { } #ModalDialog_header_line { - margin: 0px 40px 20px 40px; + margin: 0; min-height: 2px; max-height: 2px; background-color: rgba(42, 49, 157, 0.15); @@ -456,11 +456,30 @@ QWidget#FileWidget_horizontal_line { font-family: 'Montserrat'; font-size: 16px; color: #302aa3; + margin: 0; + padding: 0; +} + +#ModalDialogConfirmation { + font-family: 'Montserrat'; + font-size: 16px; + font-weight: 600; + color: #302aa3; + margin: 0; +} + +#ModalDialog.dangerous #ModalDialogConfirmation { + color: #ff3366; +} + +#ModalDialog_button_box { + border: 1px solid #ff0000; } #ModalDialog_button_box QPushButton { margin: 0px 0px 0px 12px; height: 40px; + margin: 0; padding-left: 20px; padding-right: 20px; border: 2px solid #2a319d; diff --git a/securedrop_client/resources/css/source_menu.css b/securedrop_client/resources/css/source_menu.css index 3b66959f2f..7126281269 100644 --- a/securedrop_client/resources/css/source_menu.css +++ b/securedrop_client/resources/css/source_menu.css @@ -1,17 +1,19 @@ QMenu { font-family: 'Source Sans Pro'; - padding: 1em 0; + padding: 1em 0.5em; font-size: 16px; } QMenu::separator { - padding: 1em 0.5em 1em 1em; + padding: 1em 0.5em; font-weight: 900; - color: #999; + color: #b8c0de; + letter-spacing: 0.2em; } QMenu::item { - padding: 0.5em 1em 0.5em 2em; + padding: 0.25em 1em 0.25em 1.25em; + margin: 0.25em 0 0 0; } QMenu::item:selected { diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 0da8c87285..fc032b75e4 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -4829,12 +4829,25 @@ def test_DeleteSourceDialog_make_body_text(mocker, source, session): message = delete_source_message_box.make_body_text() - expected_message = ( - "

When the entire account for a source is deleted:

" - "" - "

Are you sure this is what you want for {source}?

" + expected_message = "".join( + ( + "", + "

", + _("When the entire account for a source is deleted:"), + "

", + "

\u2219 ", + _("The source will not be able to log in with their codename again."), + "

", + "

\u2219 ", + _("Your organization will not be able to send them replies."), + "

", + "

\u2219 ", + _("All files and messages from that source will also be destroyed."), + "

", + "

 

", + ) ).format(source=source.journalist_designation) assert message == expected_message diff --git a/tests/integration/test_styles_sdclient.py b/tests/integration/test_styles_sdclient.py index 787e649b68..e90e50ea4c 100644 --- a/tests/integration/test_styles_sdclient.py +++ b/tests/integration/test_styles_sdclient.py @@ -143,9 +143,9 @@ def test_class_name_matches_css_object_name_for_modal_dialog(modal_dialog): assert "ModalDialog" in modal_dialog.body.objectName() assert "ModalDialog" in modal_dialog.body.objectName() assert "ModalDialog" in modal_dialog.continue_button.objectName() - window_buttons = modal_dialog.layout().itemAt(5).widget() + window_buttons = modal_dialog.layout().itemAt(4).widget() assert "ModalDialog" in window_buttons.objectName() - button_box = window_buttons.layout().itemAt(0).widget() + button_box = window_buttons.layout().itemAt(1).widget() assert "ModalDialog" in button_box.objectName() @@ -430,9 +430,9 @@ def test_styles_for_modal_dialog(modal_dialog): assert QFont.Bold == modal_dialog.header.font().weight() assert 24 == modal_dialog.header.font().pixelSize() assert "#2a319d" == modal_dialog.header.palette().color(QPalette.Foreground).name() - assert (4, 0, 0, 0) == modal_dialog.header.getContentsMargins() - assert 22 == modal_dialog.header_line.minimumSize().height() # 2px + 20px margin - assert 22 == modal_dialog.header_line.maximumSize().height() # 2px + 20px margin + assert (0, 0, 0, 0) == modal_dialog.header.getContentsMargins() + assert 2 == modal_dialog.header_line.minimumSize().height() # 2px + 20px margin + assert 2 == modal_dialog.header_line.maximumSize().height() # 2px + 20px margin assert 38 == math.floor(255 * 0.15) # sanity check assert 38 == modal_dialog.header_line.palette().color(QPalette.Background).rgba64().alpha8() assert 42 == modal_dialog.header_line.palette().color(QPalette.Background).red() @@ -442,7 +442,7 @@ def test_styles_for_modal_dialog(modal_dialog): assert "Montserrat" == modal_dialog.body.font().family() assert 16 == modal_dialog.body.font().pixelSize() assert "#302aa3" == modal_dialog.body.palette().color(QPalette.Foreground).name() - window_buttons = modal_dialog.layout().itemAt(5).widget() + window_buttons = modal_dialog.layout().itemAt(4).widget() button_box = window_buttons.layout().itemAt(0).widget() button_box_children = button_box.findChildren(QPushButton) for c in button_box_children: @@ -486,9 +486,9 @@ def test_styles_for_print_dialog(print_dialog): assert QFont.Bold == print_dialog.header.font().weight() assert 24 == print_dialog.header.font().pixelSize() assert "#2a319d" == print_dialog.header.palette().color(QPalette.Foreground).name() - assert (4, 0, 0, 0) == print_dialog.header.getContentsMargins() - assert 22 == print_dialog.header_line.minimumSize().height() # 2px + 20px margin - assert 22 == print_dialog.header_line.maximumSize().height() # 2px + 20px margin + assert (0, 0, 0, 0) == print_dialog.header.getContentsMargins() + assert 2 == print_dialog.header_line.minimumSize().height() # 2px + 20px margin + assert 2 == print_dialog.header_line.maximumSize().height() # 2px + 20px margin assert 38 == math.floor(255 * 0.15) # sanity check assert 38 == print_dialog.header_line.palette().color(QPalette.Background).rgba64().alpha8() assert 42 == print_dialog.header_line.palette().color(QPalette.Background).red() @@ -498,7 +498,7 @@ def test_styles_for_print_dialog(print_dialog): assert "Montserrat" == print_dialog.body.font().family() assert 16 == print_dialog.body.font().pixelSize() assert "#302aa3" == print_dialog.body.palette().color(QPalette.Foreground).name() - window_buttons = print_dialog.layout().itemAt(5).widget() + window_buttons = print_dialog.layout().itemAt(4).widget() button_box = window_buttons.layout().itemAt(0).widget() button_box_children = button_box.findChildren(QPushButton) for c in button_box_children: @@ -527,9 +527,9 @@ def test_styles_for_export_dialog(export_dialog): assert QFont.Bold == export_dialog.header.font().weight() assert 24 == export_dialog.header.font().pixelSize() assert "#2a319d" == export_dialog.header.palette().color(QPalette.Foreground).name() - assert (4, 0, 0, 0) == export_dialog.header.getContentsMargins() - assert 22 == export_dialog.header_line.minimumSize().height() # 2px + 20px margin - assert 22 == export_dialog.header_line.maximumSize().height() # 2px + 20px margin + assert (0, 0, 0, 0) == export_dialog.header.getContentsMargins() + assert 2 == export_dialog.header_line.minimumSize().height() # 2px + 20px margin + assert 2 == export_dialog.header_line.maximumSize().height() # 2px + 20px margin assert 38 == math.floor(255 * 0.15) # sanity check assert 38 == export_dialog.header_line.palette().color(QPalette.Background).rgba64().alpha8() assert 42 == export_dialog.header_line.palette().color(QPalette.Background).red() @@ -539,7 +539,7 @@ def test_styles_for_export_dialog(export_dialog): assert "Montserrat" == export_dialog.body.font().family() assert 16 == export_dialog.body.font().pixelSize() assert "#302aa3" == export_dialog.body.palette().color(QPalette.Foreground).name() - window_buttons = export_dialog.layout().itemAt(5).widget() + window_buttons = export_dialog.layout().itemAt(4).widget() button_box = window_buttons.layout().itemAt(0).widget() button_box_children = button_box.findChildren(QPushButton) for c in button_box_children: