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

[widget audit] toga.Button #1761

Merged
merged 66 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
92fd082
Whitespace change to use as the starting point for a PR.
freakboy3742 Jan 31, 2023
d206715
Add changenote.
freakboy3742 Jan 31, 2023
9fdf46e
Fix tracing on Windows
mhsmith Jan 31, 2023
38bd4d6
Migrate dummy widget tools to pytest.
freakboy3742 Feb 1, 2023
b541c09
Remove use of shadow attribute for button label.
freakboy3742 Feb 1, 2023
7077562
Convert Button tests to pytest.
freakboy3742 Feb 1, 2023
ec04ddc
Fix Android tests, get to 100% coverage.
freakboy3742 Feb 1, 2023
859ad48
Fix get_text implementation on gtk and winforms.
freakboy3742 Feb 1, 2023
f93158e
100% coverage on iOS.
freakboy3742 Feb 1, 2023
67f3374
Remove deprecated code from Button.
freakboy3742 Feb 1, 2023
ca8a760
Winforms to 100%.
freakboy3742 Feb 1, 2023
8a03783
GTK to 100%.
freakboy3742 Feb 1, 2023
37985ae
Winforms to 100%, no skips.
freakboy3742 Feb 2, 2023
2b9714e
Add test selection from the command line, and a slow mode.
freakboy3742 Feb 3, 2023
2ea497e
Fix and mark tests that aren't reliable
freakboy3742 Feb 6, 2023
c8e41b1
Updated dummy assertions to be simple functions.
freakboy3742 Feb 6, 2023
a07ebde
GTK Button color tests passing.
freakboy3742 Feb 7, 2023
e6c3096
Add a warning about UI animations.
freakboy3742 Feb 8, 2023
163fa97
Re-enable the button size test.
freakboy3742 Feb 16, 2023
8c24cbe
Correct handling of color for non-button widgets.
freakboy3742 Feb 16, 2023
df7c13e
Remove refresh calls that aren't needed any more.
freakboy3742 Feb 16, 2023
d1fdf8b
Removed the xfail marker from some tests.
freakboy3742 Feb 16, 2023
2e7f8d0
Modified the color test to assert all components in one assertion.
freakboy3742 Feb 20, 2023
da9c1ba
Use a programmatic approach to disable animations.
freakboy3742 Feb 20, 2023
50c4fee
iOS Button tests to 100%.
freakboy3742 Feb 22, 2023
4a2944f
iOS button to 100%.
freakboy3742 Feb 24, 2023
fe92bb9
Partial fixes for Android colors and fonts.
freakboy3742 Feb 24, 2023
73f0c5a
Implement `await redraw` for Android
mhsmith Feb 26, 2023
2ff0e95
Fix Android font size calculation
mhsmith Feb 26, 2023
e6ed1c1
Fix button background color
mhsmith Feb 27, 2023
db63ecc
Modify font size and font handling.
freakboy3742 Feb 28, 2023
493e722
Add support for resetting foreground color.
freakboy3742 Feb 28, 2023
c3d5c8a
Ensure size probes return SP not DP.
freakboy3742 Feb 28, 2023
542faf7
Use del rather than assigning None to clear style.
freakboy3742 Feb 28, 2023
d3581ec
Don't allow newlines in button text.
freakboy3742 Feb 28, 2023
e1458ba
Correct label color tests.
freakboy3742 Feb 28, 2023
32451bd
Button at 100% on all tested platforms!
freakboy3742 Feb 28, 2023
c788a8c
Rework the EventLog so it is global.
freakboy3742 Feb 28, 2023
da5105a
Disallow empty labels on buttons.
freakboy3742 Feb 28, 2023
5bf1cbb
Revert a libs restructure.
freakboy3742 Feb 28, 2023
29924f0
More tweaks to the dummy test assertions.
freakboy3742 Feb 28, 2023
7e2bc2d
Removed a redundant background color implementation.
freakboy3742 Feb 28, 2023
8211f8b
Add notes about button text restrictions.
freakboy3742 Feb 28, 2023
8cd8531
Merge branch 'main' into audit-button
freakboy3742 Mar 5, 2023
857d899
Change `= None` tests to `del`, for compatibility with travertino mai…
mhsmith Mar 13, 2023
7bd87a5
Fix Android font size, and add font size reset test
mhsmith Mar 13, 2023
16ec542
Add memory retention for cached font instances.
freakboy3742 Mar 13, 2023
8bd9528
Merge branch 'system-ci' into audit-button
mhsmith Mar 14, 2023
e83f2fa
Allow default font to be returned as SANS_SERIF as well as SYSTEM
mhsmith Mar 14, 2023
81e2d29
Add examples/font_size
mhsmith Mar 14, 2023
02a345d
Add bold and italic font tests (working on Android, Cocoa and iOS)
mhsmith Mar 16, 2023
9248973
Add bold and italic font tests (working on GTK and Winforms)
mhsmith Mar 17, 2023
45e52e6
Remove redundant set_font methods in dummy widgets
mhsmith Mar 17, 2023
8445dae
Merge branch 'main' into audit-button
freakboy3742 Mar 20, 2023
c646ef0
Improve comments
mhsmith Mar 20, 2023
e2da16b
Move generic font mapping assertions to tests_backend (Android, Cocoa…
mhsmith Mar 20, 2023
926c3d6
Move generic font mapping assertions to tests_backend (GTK)
mhsmith Mar 20, 2023
7f96efa
Move generic font mapping assertions to tests_backend (Winforms)
mhsmith Mar 20, 2023
36ed554
Fix typos
mhsmith Mar 20, 2023
091e3ec
Allow Button text to be set to an empty string, and make Winforms dis…
mhsmith Mar 20, 2023
dd73a9b
Use subclassing rather than an isinstance check.
freakboy3742 Mar 20, 2023
cfdc3e5
Revert the use of an interface state variable for button text.
freakboy3742 Mar 21, 2023
ce54201
Mark some classes with Test in the name as not-tests.
freakboy3742 Mar 21, 2023
5abe2f1
Tweak section heading.
freakboy3742 Mar 21, 2023
f76c90a
Modify the winforms button probe to do ZWS normalization.
freakboy3742 Mar 21, 2023
8f55716
Use correct zero width space character, and remove unnecessary workar…
mhsmith Mar 21, 2023
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
3 changes: 0 additions & 3 deletions android/src/toga_android/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ def create(self):
self.native.setOnClickListener(TogaOnClickListener(button_impl=self))
self.cache_textview_defaults()

def get_text(self):
return str(self.native.getText())

def set_text(self, text):
self.native.setText(text)

Expand Down
3 changes: 0 additions & 3 deletions cocoa/src/toga_cocoa/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ def set_font(self, font):
# Button style is sensitive to font changes
self._set_button_style()

def get_text(self):
return str(self.native.title)

def set_text(self, text):
self.native.title = text

Expand Down
25 changes: 16 additions & 9 deletions core/src/toga/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,26 @@ def __init__(

@property
def text(self):
"""The text displayed on the button."""
return self._impl.get_text()
"""The text displayed on the button.

``None`` will be displayed as an empty string. Any other object will be converted
to a string using ``str()``.

Only one line of text can be displayed. Any content after the first newline will
be ignored.
"""
return self._text

@text.setter
def text(self, value):
if not value:
raise ValueError("Button must have a label")

# Button text can't include line breaks. Strip any content
# after a line break (if provided)
value = str(value).split("\n")[0]
if value is None:
self._text = ""
else:
# Button text can't include line breaks. Strip any content
# after a line break (if provided)
self._text = str(value).split("\n")[0]

self._impl.set_text(value)
self._impl.set_text(self._text)
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm generally against the introduction of interface state/shadow variables; in this case, the shadow variable is only required if we have problems round-tripping the original value. In this implementation we're not round tripping anyway (because we're losing None and multiline input); to support the winforms edge case, I'm entirely comfortable with a normalizing a user-supplied "\u0200" (zero width space) to ""). It's highly unlikely to ever be user supplied in that form, and it's functionally indistinguishable from "".

Copy link
Member

Choose a reason for hiding this comment

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

I'm entirely comfortable with a normalizing a user-supplied "\u0200" (zero width space) to "").

Makes sense, but \u0200 is LATIN CAPITAL LETTER A WITH DOUBLE GRAVE. I've replaced that with \u200B.

self.refresh()

@property
Expand Down
28 changes: 2 additions & 26 deletions core/tests/widgets/test_button.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
import toga
from toga_dummy.utils import (
EventLog,
assert_action_not_performed,
assert_action_performed,
assert_action_performed_with,
assert_attribute_not_set,
attribute_value,
)

Expand All @@ -27,6 +25,8 @@ def test_widget_created(button):
"value, expected",
[
("New Text", "New Text"),
("", ""),
(None, ""),
(12345, "12345"),
("Contains\nnewline", "Contains"),
],
Expand All @@ -48,30 +48,6 @@ def test_button_text(button, value, expected):
assert_action_performed(button, "refresh")


@pytest.mark.parametrize(
"value",
[
None,
"",
],
)
def test_empty_button_text(button, value):
"""The button label cannot be set to empty text."""
assert button.text == "Test Button"

# Clear the event log
EventLog.reset()

with pytest.raises(ValueError, match=r"Button must have a label"):
button.text = value

# test backend has not changed value
assert_attribute_not_set(button, "text")

# No refresh was performed
assert_action_not_performed(button, "refresh")


def test_button_on_press(button):
"""The on_press handler can be invoked."""
# No handler initially
Expand Down
10 changes: 2 additions & 8 deletions docs/reference/api/widgets/button.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,8 @@ A button has a text label. A handler can be associated with button press events.

button = toga.Button('Click me', on_press=my_callback)

Notes
Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should keep this as a "Notes" section. While the notes at present are all style related, we're very likely going to come across platform specific eccentricities that don't fit the description being "style", and don't map to a specific attribute like the text handling of None et al.

-----
* Any "truthy" object (i.e, object that evaluates as ``True``) can be provided
as the text for the button; if the object is not a string, it will be
automatically converted to a string using ``str()``.

* The button text cannot contain newlines. Any content after the first newline
will be ignored.
Copy link
Member

Choose a reason for hiding this comment

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

I've moved the property documentation into the docstring so it'll appear next to the property itself, rather than in a separate section.

Styles
------

* A background color of ``TRANSPARENT`` will be treated as a reset of the button
to the default system color.
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/api/widgets/label.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Usage

label = toga.Label('Hello world')

Notes
-----
Styles
------

* Winforms does not support an alignment value of ``JUSTIFIED``. If this
alignment value is used, the label will default to left alignment.
Expand Down
3 changes: 0 additions & 3 deletions dummy/src/toga_dummy/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@ def create(self):
def set_text(self, text):
self._set_value("text", text)

def get_text(self):
return self._get_value("text")

def set_on_press(self, handler):
self._set_value("on_press", handler)
3 changes: 0 additions & 3 deletions gtk/src/toga_gtk/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ def create(self):
self.native.connect("show", lambda event: self.refresh())
self.native.connect("clicked", self.gtk_on_press)

def get_text(self):
return self.native.get_label()

def set_text(self, text):
self.native.set_label(text)

Expand Down
3 changes: 0 additions & 3 deletions iOS/src/toga_iOS/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ def create(self):
# Add the layout constraints
self.add_constraints()

def get_text(self):
return str(self.native.titleForState(UIControlStateNormal))

def set_text(self, text):
self.native.setTitle(text, forState=UIControlStateNormal)

Expand Down
2 changes: 1 addition & 1 deletion testbed/tests/data.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from toga.colors import rgba

TEXTS = [" ", "a", "ab", "abc", "hello world", "hello\nworld", "你好, wørłd!"]
TEXTS = ["", " ", "a", "ab", "abc", "hello world", "hello\nworld", "你好, wørłd!"]


COLORS = [
Expand Down
24 changes: 0 additions & 24 deletions testbed/tests/widgets/properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,6 @@ async def test_text(widget, probe):
assert probe.text == text


async def test_text_empty(widget, probe):
"The text displayed on a widget can be empty"
# Set the text to the empty string
widget.text = ""
Copy link
Member

Choose a reason for hiding this comment

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

Empty strings are now covered by test_text above, and conversion of non-string objects is covered by test_button.py in the core.

await probe.redraw()

assert widget.text == ""
assert probe.text == ""

# Reset back to "actual" content
widget.text = "Hello"
await probe.redraw()

assert widget.text == "Hello"
assert probe.text == "Hello"

# Set the text to None; renders as an empty string
widget.text = None
await probe.redraw()

assert widget.text == ""
assert probe.text == ""


async def test_text_width_change(widget, probe):
"If the widget text is changed, the width of the widget changes"
orig_width = probe.width
Expand Down
13 changes: 11 additions & 2 deletions testbed/tests/widgets/test_button.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,17 @@ async def test_text(widget, probe):
await probe.redraw()

# Text after a newline will be stripped.
assert widget.text == text.split("\n")[0]
assert probe.text == text.split("\n")[0]
expected = text.split("\n")[0]
assert widget.text == expected
try:
assert probe.text == expected
except AssertionError:
if probe.text == " " and expected == "":
# The Winforms backend avoids empty labels to prevent the widget height
# from collapsing.
pass
else:
raise

assert probe.height == initial_height

Expand Down
1 change: 0 additions & 1 deletion testbed/tests/widgets/test_label.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
test_font,
test_font_attrs,
test_text,
test_text_empty,
test_text_width_change,
)

Expand Down
3 changes: 0 additions & 3 deletions web/src/toga_web/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ def dom_onclick(self, event):
if self.interface.on_press:
self.interface.on_press(self.interface)

def get_text(self):
return self.native.innerHTML

def set_text(self, text):
self.native.innerHTML = text

Expand Down
7 changes: 4 additions & 3 deletions winforms/src/toga_winforms/widgets/button.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ def winforms_click(self, sender, event):
if self.interface.on_press:
self.interface.on_press(self.interface)

def get_text(self):
return self.native.Text

def set_text(self, text):
if text == "":
# An empty label would cause the widget's height to collapse, so display a
# single space instead.
text = " "
self.native.Text = text

def set_font(self, font):
Expand Down