[widget audit] TextInput, PasswordInput #1944

freakboy3742 · 2023-05-18T06:01:21Z

Audit of both TextInput and PasswordInput (since they're essentially the same widget).

Extends #1938, as there's a lot of shared functionality in the probe.

There are some backwards incompatible changes in the API of individual Validators. Since it's a fairly niche feature, that wasn't previously documented (although there was example code), I didn't think it was worth a full backwards compatibility path.
Modifies the implementation of validation on macOS so that it doesn't require modal dialogs
Adds confirmation handling on macOS
Adds an implementation of focus handling, confirmation handling and validation on iOS.

Fixes #1290.
Fixes #1315.

Audit checklist

freakboy3742 · 2023-05-19T00:10:01Z

#1290 and #1315 should be fixed by this PR.

…ize.

…xtInput.

mhsmith · 2023-05-26T20:53:33Z

pyproject.toml

+    { directory = "feature", name = "Features", showcontent = true },
+    { directory = "bugfix", name = "Bugfixes", showcontent = true },
+    { directory = "removal", name = "Backward Incompatible Changes", showcontent = true },


Shouldn't backward incompatible changes come first?

I guess my take would be that you talk about the awesome new stuff before you scare people with the things that have changed :-)

As a comparable example - Django's release notes list major features, minor features, then backwards incompatibilities.

docs/reference/api/resources/validators.rst

core/src/toga/validators.py

mhsmith · 2023-05-28T15:50:11Z

core/src/toga/widgets/textinput.py

+    def validate(self) -> bool:
+        """Validate the current value of the widget.


Since validation happens automatically on both programmatic and user-generated changes, is there any reason for this method to be public?

No particular reason for it to be public (other than, at this point, backwards compatibility). At the very least, it's worth flagging in docs that end-users shouldn't ever need to call this method; but I wouldn't be opposed to making it an internal method (with the appropriate backwards compatibility note).

core/src/toga/validators.py

mhsmith · 2023-05-28T23:50:32Z

winforms/src/toga_winforms/widgets/textinput.py

-    def winforms_double_click(self, sender, event):
-        self.native.SelectAll()


I don't know why this was here: double-clicking on Windows is supposed to select one word, not the entire box.

Yeah - I can't explain this one either.

mhsmith · 2023-05-28T23:51:22Z

winforms/src/toga_winforms/widgets/textinput.py

-    def winforms_validated(self, sender, event):
-        self.interface.validate()


I can't work out what the Validated event handler was supposed to do, but everything seems to work fine without it.

Winforms provides explicit event hooks to perform validation independent of focus and text change; I think the intention is that you can prevent a widget from losing focus if it's not currently valid. However, we're not blocking focus in that way, so I guess we can just piggyback on the change event.

mhsmith · 2023-05-28T23:54:19Z

winforms/tests_backend/widgets/textinput.py

+    def assert_height(self, min_height, max_height):
+        # Height isn't configurable in this native widget.
+        assert 12 <= self.height <= 22


The native TextBox seems to ignore its height allocation and always stay the same height. Does this happen with any other widgets? I think I remember us talking about it, but I can't find any similar implementations of this method.

There's some similar behavior with Button. Buttons don't expand to fill the available vertical space unless they're explicitly set to a given height.

There's also some similar (but inconsistent) behavior with Switch. On most platforms, the switch "graphic" is a fixed height, regardless of the containing box; however, GTK and iOS both violate this, and render the switch with a weird aspect ratio.

In this case, I'm happy to call this a platform eccentricity and document it.

Co-authored-by: Malcolm Smith <smith@chaquo.com>

mhsmith

@freakboy3742: All platforms now have 100% coverage, and a fully-working textinput example. Please review my last few commits, and merge the PR if you're happy.

freakboy3742 · 2023-05-29T23:22:01Z

@mhsmith One minor tweak (avoiding the use of ObjC invocations for validation), but otherwise this all looks good to me.

freakboy3742 marked this pull request as draft May 18, 2023 06:02

freakboy3742 mentioned this pull request May 19, 2023

implementations should use interface values (vs raw) #1315

Closed

freakboy3742 force-pushed the audit-textview branch 2 times, most recently from b69882a to 2b96f4d Compare May 22, 2023 03:30

freakboy3742 mentioned this pull request May 22, 2023

[widget Audit] toga.NumberInput #1946

Merged

11 tasks

freakboy3742 added 21 commits May 24, 2023 14:28

Documented and updated tests for validators.

5c6bf3a

Complete docstrings for TextInput.

9dc20b9

Core tests ported to pytest.

cbc4afe

Update documentation and tests for PasswordInput.

eade191

Cocoa TextInput and PasswordInput to 100%.

6299d15

Update API index page.

71c6214

Update on_change handler to include programmatic changes.

6ed6a9d

iOS tests to 100%

7ddb0b4

Add changenotes.

aceb76b

Add Towncrier config for backwards incompatible changes.

f351e58

Correct spelling in validation docs.

78a376b

Correct docstring markup error.

ae7006f

Complete implementation of GTK TextInput/PasswordInput.

0f3d937

Tweak GTK libs to get coverage.

2ccbd40

Tweaks to docs to fix spelling and clarify API.

2dfaea2

Restore winforms to working status.

7829c99

Add placeholder fix for vertical alignment.

52bf75c

Correct GTK wrapper import.

c942ca4

Complete removal of clear() method.

540c83b

Correct merge problems with GTK justification and scrollable widget s…

4a0999d

…ize.

Reworked vertical alignment tests to allow for CENTER alignment on Te…

43b0fe7

…xtInput.

freakboy3742 force-pushed the audit-textview branch from aee109b to 0fc3141 Compare May 24, 2023 06:50

Correct changes from rebase with Multilinetext audit.

b3bba0d

freakboy3742 force-pushed the audit-textview branch from 0fc3141 to b3bba0d Compare May 24, 2023 06:54

mhsmith changed the title ~~[widget audit] toga.TextField, toga.PasswordField~~ [widget audit] TextInput, PasswordInput May 24, 2023

Merge branch 'main' into audit-textview

2034d08

freakboy3742 marked this pull request as ready for review May 25, 2023 22:32

Enable future annotations.

b22c064

freakboy3742 force-pushed the audit-textview branch from 9afbfb8 to b22c064 Compare May 26, 2023 06:03

mhsmith self-requested a review May 26, 2023 09:09

mhsmith added 2 commits May 28, 2023 17:13

Split 2-line change note into separate entries

3065ff6

TextInput example app cleanups

c48992e

mhsmith reviewed May 28, 2023

View reviewed changes

Move default verify_... fixtures to conftest.py

e000576

mhsmith reviewed May 28, 2023

View reviewed changes

core/src/toga/validators.py Outdated Show resolved Hide resolved

core/src/toga/validators.py Outdated Show resolved Hide resolved

core/src/toga/validators.py Outdated Show resolved Hide resolved

Winforms at 100% coverage

26837cd

mhsmith reviewed May 28, 2023

View reviewed changes

freakboy3742 and others added 7 commits May 29, 2023 09:11

Documentation tweaks from code review.

ada227e

Co-authored-by: Malcolm Smith <smith@chaquo.com>

Update validator APIs to remove unneeded allow_empty arguments.

7c066f3

Added notes about Winforms platform discrepancies.

c2a841f

Correct widget references in PasswordInput docs.

cd58668

Correct typo in switch docs.

0149c78

Make validate method private, and remove some redundant calls to it

3e421ab

Add tests for value being hidden in PasswordInput

d6fdb1c

mhsmith force-pushed the audit-textview branch from 716b779 to d6fdb1c Compare May 29, 2023 15:37

mhsmith added 3 commits May 29, 2023 21:18

Android at 100% coverage

f5acce2

Fix CI issue with TextInputProbe.type_character on Android

b531b7f

Android: mark uncovered branches for unknown keycodes

bcd813b

mhsmith approved these changes May 29, 2023

View reviewed changes

Avoid using ObjC calls to invoke validation.

e634597

freakboy3742 merged commit 488b417 into beeware:main May 29, 2023

freakboy3742 deleted the audit-textview branch May 29, 2023 23:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[widget audit] TextInput, PasswordInput #1944

[widget audit] TextInput, PasswordInput #1944

freakboy3742 commented May 18, 2023 •

edited by mhsmith

Loading

freakboy3742 commented May 19, 2023

mhsmith May 26, 2023

freakboy3742 May 29, 2023

mhsmith May 28, 2023

freakboy3742 May 29, 2023

mhsmith May 28, 2023

freakboy3742 May 29, 2023

mhsmith May 28, 2023

freakboy3742 May 29, 2023

mhsmith May 28, 2023

freakboy3742 May 29, 2023

mhsmith left a comment

freakboy3742 commented May 29, 2023

		def validate(self) -> bool:
		"""Validate the current value of the widget.

		def winforms_double_click(self, sender, event):
		self.native.SelectAll()

		def winforms_validated(self, sender, event):
		self.interface.validate()

[widget audit] TextInput, PasswordInput #1944

[widget audit] TextInput, PasswordInput #1944

Conversation

freakboy3742 commented May 18, 2023 • edited by mhsmith Loading

Audit checklist

freakboy3742 commented May 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhsmith left a comment

Choose a reason for hiding this comment

freakboy3742 commented May 29, 2023

freakboy3742 commented May 18, 2023 •

edited by mhsmith

Loading