Widget API uniformisation [breaking changes ahead]

[bruno-rino]

I did a sweep through all the widgets to try to find inconsistencies. This is what I found. I know a lot of my suggestions boil down to personal preference, but I hope this make sense to more people.

"initial value" constructor parameter

The name of the constructor parameter to set an initial value is not uniform. Some use initial, others default, Switch uses is_on. I would expect this parameter to be called value, like the property (except maybe Switch; more below).

Widgets: TextInput, PasswordInput, NumberInput, MultilineTextInput, Slider, DatePicker, TimePicker, Switch

Button and Label text

It's Button.label and Label.text. I would suggest they should both be text

Switch

This widget is an oddball.

The value property is called is_on instead of value. It feels wrong for another reason: instinctively, I expect a property prefixed by is_ to be read-only.

The change event is called on_toggle. It should be on_change.

[freakboy3742]

Generally speaking, I agree consistency in APIs is something we should be aiming for; and these suggestions pretty much all point in the right direction. One of the things that has been causing me to hold off making 0.3 final has been nailing down API inconsistencies like these, so thanks for the prompt to take a closer look at this problem.

PRs (or tickets describing the nature of the change) would all be welcome, within the caveats listed below. For backwards compatibility, any naming change should preserve the existing API, logging a deprecation warning if the old API is used, and clearly marking out pieces of code that can be purged when the time comes to remove the backwards compatibility shim.

Some specific comments:

Initial value

I agree that value is the appropriate argument to use for the initial value of a widget. It doesn't make much sense to use initial and introduce a naming difference between the "starting" and "current" value of a widget.

There is a very subtle difference with default, though. A default is the value that the widget will return to when cleared; and that won't necessarily be the initial starting value. NumberInput is the place where this is most obvious. Take an example of a thermostat - an app might set the initial value of the thermostat to be whatever the user had previously set as their temperature preference, but the default value would be whatever the app determines to be "room temperature" when the user resets the widget content.

Similarly, some widgets have a placeholder; this is text that will be displayed if the widget is empty, but won't be returned as active content of the widget. A widget shouldn't have both placeholder and default (or, at least, they shouldn't be able to specify both).

Button and label text

I agree that text is a better attribute for setting text content. Label may eventually acquire a html attribute as well, allowing for "rich text labels"; .text is thematically consistent with .html, so that would seem to support the choice.

Switch

Switch.is_on is a bit of an outlier; it's useful in the sense of a 'human interface' for code, as if my_switch.is_on: reads in English a little better than if my_switch.value:. However, consistency is a useful property for any API to have, so I think I agree that value is a better. I also agree that on_change is a more appropriate handler for changes than on_toggle

[bruno-rino]

I understand your point regarding default. However I fail to see how it applies to the existing widgets (NumberInput and Slider): how do you clear their value?

[freakboy3742]

It depends on the platform and the widget; Slider definitely can't be reset, but with NumberInput on desktop, you can usually highlight the input content, press delete, and then shift focus to another widget.

[bruno-rino]

So these are done. But it turns out I wasn't as thorough as I though. There is one more renaming that could be done.
This is less annoying that the other inconsistencies, but still.

Command and OptionItem both have label properties that are just strings. At the very first glance, the name of the property might induce one to think the property contains a toga.Label object.

That's it. What do you think? I feel a bit pedantic even suggesting it. But still.

[mhsmith]

Yes, these also look like good candidates for changing to text.

[freakboy3742]

I agree - text would make sense for these as well. They won't ever have a .html property like Label might eventually acquire; but I'd argue that's appropriate pseudo-documentation - menu items can only be text labels, they can't have HTML adornment.