Skip to content

Commit

Permalink
Refactor LoginOfflineButton as QPushButton
Browse files Browse the repository at this point in the history
The Qt buttons do integrate with the operating system, as a result
they are easier to take advantage of for accessibility purposes.

This is a minimal modification: the button behavior has not changed,
even though it could be improved in a few aspects (e.g. implementing
some of the conventional button states would allow to provide timely
feedback to the person interacting with the button).

Decisions:

- I called the base class SDPushButton by analogy with QPushButton.
  The intention is for that class to provide a generic button, adequate
  to the SecureDrop Client context.
- I'd argue that LoginOfflineLink wasn't a link - understood as
  hyperlink. Links come with expectations of behavior (e.g. history)
  that are not met in this case. On the other hand, the behavior of
  LoginOffineLink was close to that of a dialog "dismiss" button.
- I didn't write any test for SDPushButton because it doesn't implement
  any behavior that was tested in LoginOfflineLink. In other words,
  the button is not less tested as it is.
- I inlined the stylesheet in the component for two reasons:
  1. It is meant to be a generic component and no other, or few other
     buttons should ever need to override styles or behavior.
     If the styles cover all buttons, then having them scoped to the
     button's namespace and close to the button's code seems
     preferrable to having them in a (large) single stylesheet.
  2. QSS doesn't support CSS custom properties, but Python f-strings
     can be used to make intentions clearer.

See https://material.io/components/buttons
and https://speakerdeck.com/didoo/let-there-be-peace-on-css?slide=62
  • Loading branch information
gonzalo-bulnes committed Nov 9, 2021
1 parent 5fdcd71 commit a82cd00
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 37 deletions.
2 changes: 2 additions & 0 deletions securedrop_client/gui/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from PyQt5.QtCore import QSize, Qt
from PyQt5.QtWidgets import QHBoxLayout, QLabel, QPushButton, QWidget

# Allow the buttons to be imported directly.
from securedrop_client.gui.buttons import SDPushButton # noqa: F401
from securedrop_client.resources import load_icon, load_svg


Expand Down
47 changes: 47 additions & 0 deletions securedrop_client/gui/buttons.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""
SecureDrop Buttons
These are appropriate for use in the SecureDrop Client.
These buttons look and behave appropriately in the context of SecureDrop client,
but besides that, they are regular Qt buttons which means they benefit from
the full-range of accessibility features provided by the framework.
When adding or extending buttons in this file, please make sure the implementation
doesn't obstruct the default Qt buttons API.
Copyright (C) 2021 The Freedom of the Press Foundation.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as published
by the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
"""
from typing import NewType

from PyQt5.QtWidgets import QPushButton

from securedrop_client.resources import load_css


class SDPushButton(QPushButton):
"""A QPushButton that follows SecureDrop guidelines."""

Alignment = NewType("Alignment", str)
AlignLeft = Alignment("left-aligned")

def __init__(self) -> None:
super().__init__()
self.setStyleSheet(load_css("button.css"))

def setAlignment(self, align: Alignment) -> None:
"""Visually align a button that doesn't have a visible outline."""
self.setProperty("class", f"button text {align}")
27 changes: 10 additions & 17 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@
User,
)
from securedrop_client.export import ExportError, ExportStatus
from securedrop_client.gui import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton
from securedrop_client.gui import (
SDPushButton,
SecureQLabel,
SvgLabel,
SvgPushButton,
SvgToggleButton,
)
from securedrop_client.logic import Controller
from securedrop_client.resources import load_css, load_icon, load_image, load_movie
from securedrop_client.storage import source_exists
Expand Down Expand Up @@ -1576,26 +1582,13 @@ def on_star_update_successful(self, source_uuid: str) -> None:
self.pending_count = self.pending_count - 1


class LoginOfflineLink(QLabel):
"""
A button that logs the user in in offline mode.
"""

clicked = pyqtSignal()
class LoginOfflineLink(SDPushButton):
"""A button that logs the user in, in offline mode."""

def __init__(self) -> None:
# Add svg images to button
super().__init__()

# Set css id
self.setObjectName("LoginOfflineLink")

self.setFixedSize(QSize(120, 22))

self.setText(_("USE OFFLINE"))

def mouseReleaseEvent(self, event: QMouseEvent) -> None:
self.clicked.emit()
self.setAlignment(SDPushButton.AlignLeft)


class SignInButton(QPushButton):
Expand Down
17 changes: 17 additions & 0 deletions securedrop_client/resources/css/button.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
.button {
color: #ffffff;
font-family: "Montserrat";
font-weight: 500;
font-size: 13px;
padding: 11px 18px;
}

.text {
border: none;
background-color: none;
text-decoration: underline;
}

.left-aligned {
margin: 0 0 0 -18px;
}
6 changes: 0 additions & 6 deletions securedrop_client/resources/css/sdclient.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ QWidget {
color: #9fddff;
}

#LoginOfflineLink {
border: none;
color: #fff;
text-decoration: underline;
}

#SignInButton {
border: none;
background-color: #05edfe;
Expand Down
13 changes: 0 additions & 13 deletions tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
LoginButton,
LoginDialog,
LoginErrorBar,
LoginOfflineLink,
MainView,
MessageWidget,
PasswordEdit,
Expand Down Expand Up @@ -2747,18 +2746,6 @@ def test_LoginErrorBar_clear_message(mocker):
error_bar.hide.assert_called_with()


def test_LoginOfflineLink(mocker):
"""
Assert that the clicked signal is emitted on mouse release event.
"""
offline_link = LoginOfflineLink()
offline_link.clicked = mocker.MagicMock()

offline_link.mouseReleaseEvent(None)

offline_link.clicked.emit.assert_called_with()


def test_LoginDialog_closeEvent_does_not_exit_when_main_window_is_visible(mocker):
"""
If the main window is visible, then to not exit the application when the LoginDialog receives a
Expand Down
1 change: 0 additions & 1 deletion tests/integration/test_styles_sdclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ def test_class_name_matches_css_object_name(mocker, main_window):
assert "LoginDialog" in app_version_label.objectName()
login_offline_link = login_dialog.offline_mode
assert "LoginOfflineLink" == login_offline_link.__class__.__name__
assert "LoginOfflineLink" == login_offline_link.objectName()
login_button = login_dialog.submit
assert "SignInButton" == login_button.__class__.__name__
assert "SignInButton" in login_button.objectName()
Expand Down

0 comments on commit a82cd00

Please sign in to comment.