[widget Audit] toga.OptionContainer #1996
Conversation
freakboy3742
force-pushed
the
audit-optioncontainer
branch
2 times, most recently
from
9a38b7f
to
19ab0a5
Compare
freakboy3742
force-pushed
the
audit-optioncontainer
branch
from
df78011
to
58bdf2e
Compare
freakboy3742
force-pushed
the
audit-optioncontainer
branch
from
e625638
to
76e8887
Compare
| autodoc_default_options = { | ||
| "members": True, | ||
| "undoc-members": True, | ||
| } |
There was a problem hiding this comment.
I've removed these default options from the other containers and the Widget base class: we can clean up the others in a separate PR.
| def __iter__(self): | ||
| """Obtain an iterator over all tabs in the OptionContainer.""" | ||
| return iter(self._options) |
There was a problem hiding this comment.
Every class with __len__ and __getitem__ automatically supports iteration.
| raise self.interface.OptionException( | ||
| "Currently selected option cannot be disabled" | ||
| ) | ||
|
|
||
| self._disabled_tabs.add(index) | ||
| tabview._setTabEnabled(enabled) |
There was a problem hiding this comment.
There was a problem hiding this comment.
My feeling is that if there are some undocumented methods which have worked all this time, then we might as well keep on using them, but wrap them with broad exception handlers in case the methods ever change or disappear in a future version of macOS. That way, we'll still discover such problems as soon as we start running the testbed on the new macOS version, but existing Toga-based apps will only display a warning rather than crash.
There was a problem hiding this comment.
Yeah - this is weird. I'm not sure how anyone found the original _setTabEnabled method. I've added 2 layers of protection: catching the AttributeError in case the method disappears; and implementing the (missing, but referenced) delegate method that is strictly responsible for determining whether the tab can be selected.
| @@ -4,7 +4,20 @@ | |||
| from toga_winforms.libs import Color, Point, Size, SystemColors | |||
|
|
|||
|
|
|||
| class Widget: | |||
| class Scalable: | |||
There was a problem hiding this comment.
Any particular reason for making a subclass for this? It's only ever subclassed by Widget, and it's not used as a basis for isolated testing; I'm not sure I see the benefit of a separate base class for this.
There was a problem hiding this comment.
It's also subclassed by Window, and used in the position and size methods. This was required by the OptionContainer tests, because they rely on the window being 640x480 CSS pixels.
There was a problem hiding this comment.
Winforms and docs changes all look good for me, with one minor query about the extra base class; personal preference would be to not introduce the complexity, but I don't feel that strongly about it.
I've also implemented the missing cocoa tab_enabled probe method.
Happy for this to be merged if you are.
Audit of OptionContainer.
Builds on #1984 because of the changes to root containers on Cocoa.
This is a widget that is going to have some weird platform inconsistencies.
Removes the ability to increment and decrement the current tab. This was primarily required by typing, so that there is a consistent
OptionItemtype; but it's also a little odd to havecurrent_tabreturn anOptionItemobject, but accept integer increments.Removes the
add(),append()andinsert()APIs from OptionContainer because they collide with the child management APIs. Equivalent APIs exist on thecontentobject.Removes the
optionargument from theon_selectcallback.There's one very nasty hack in the Cocoa probe; details in the probe implementation.
There's another hack introduced into the unrelated TextInput tests; I was seeing an intermittent (but annoyingly, usually only when I was running the whole test suite) failure because the keyboard event handler wasn't firing.
Fixes #1968.
Audit checklist