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

Modify invocation order of TextInput on_change and validation #2325

Closed
freakboy3742 opened this issue Jan 5, 2024 Discussed in #2323 · 3 comments · Fixed by #2596
Closed

Modify invocation order of TextInput on_change and validation #2325

freakboy3742 opened this issue Jan 5, 2024 Discussed in #2323 · 3 comments · Fixed by #2596
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!

Comments

@freakboy3742
Copy link
Member

freakboy3742 commented Jan 5, 2024

Discussed in #2323

Via @cozimus

Originally posted by cozimus January 4, 2024
In this minimum working example I tried to enable/disable a button based on a is_valid condition on a TextInput:

import toga
from toga.validators import Number

class MainApp(toga.App):
    def startup(self):
        self.main_window = toga.MainWindow(title=self.formal_name)
        self.button = toga.Button("submit")
        text_input = toga.TextInput(on_change=self.set_button, validators=[Number()])
        self.main_window.content = toga.Box(children=[text_input, self.button])
        self.main_window.show()

    def set_button(self, widget: toga.TextInput):
        self.button.enabled = widget.is_valid


def main():
    return MainApp("App", "com.example")


if __name__ == "__main__":
    main().main_loop()

My goal is to disable the button whenever the text is not valid.
However when I modify the text with a non-number value, the button is still enabled, and it is disabled only when I change again the text.
It seems like the on_change handle is triggered before the is_valid flag update, hence when I call my set_button functions, is_valid is still as the previous state.

This should be the behavior of validation; the current behavior isn't intentional (or documented as such).

Suggested implementation

  • Refactor the pair of on_change() and _validate() calls used by every backend into a single (private) _value_changed() method on the core class that can be invoked by backends whenever the value changes.
  • Add a test to the core API tests that confirms that when the on_change() handler is invoked, the is_valid status is correct. This validates that the core API provides utilities that validate in the correct order.
  • Add a test to the test backend that validates the same. This confirms that all the backends are either using the new private method, or at least have an implementation that is consistent.
  • Update the documentation for on_change to explicitly define that the field will have been validated when on_change is invoked.
@freakboy3742 freakboy3742 added enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start! labels Jan 5, 2024
@gocaza
Copy link

gocaza commented Jan 8, 2024

wouldn't it help just to invert the order the functions are called inside the core modules? I tried this and it seems to work
image

@freakboy3742
Copy link
Member Author

Yes- that would, indeed fix the problem- and would be part of the first bullet point in the suggested implementation. I've suggested that refactoring is worthwhile because this is functionality that is common to every backend, so we can ensure that if any other change-related behavior occurs (or a new platform is added), the behavior will be consistent.

@PistachioCannoli
Copy link
Contributor

I'll look into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants