Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve handling of <Destroy> event in TopLevelDialogs #683

Open
windymilla opened this issue Jan 19, 2025 · 0 comments · May be fixed by #695
Open

Improve handling of <Destroy> event in TopLevelDialogs #683

windymilla opened this issue Jan 19, 2025 · 0 comments · May be fixed by #695
Assignees
Labels
code improvement Improve maintainability, consistency, etc

Comments

@windymilla
Copy link
Collaborator

While reviewing #579, I noticed that the <Destroy> event was bound to the top_frame instead of the dialog. Further investigation reveals that this avoids the problem of the bound method being called once for every widget in the dialog (explanation below for future reference).

Suggestion:

  1. In widgets.py, bind tidy_up to self.top_frame rather than self
  2. In tidy_up there's no longer any need to check the widget class, since it will just be called once
  3. Similarly in checkers.py, the tidy_up method doesn't need to check widget class (Also fix comment at top of method)
  4. Consider renaming tidy_up to something with destroy in it
  5. In search.py (once Showing all matches during a search #579 is merged), instead of binding to destroy, override the tidy_up method (like checkers.py)

Explanation of difference between binding to dialog and binding to top_frame:
(Taken from here)

  1. Every widget has a set of 4 binding tags: itself, its class, its toplevel window, all
  2. When you bind, you're binding to a tag, not to a widget
  3. Therefore, if you pass top_frame (not a toplevel window) into the binding routine, it only matches the "itself" tag of the top_frame widget.
  4. However, if you pass in my_dialog (a toplevel window), it matches not only the "itself" tag of the my_dialog widget, but also the toplevel window tag of all of its child widgets. So, you appear to have bound your method to the dialog and all its children.
@windymilla windymilla added the code improvement Improve maintainability, consistency, etc label Jan 19, 2025
@windymilla windymilla self-assigned this Jan 20, 2025
@windymilla windymilla linked a pull request Jan 25, 2025 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
code improvement Improve maintainability, consistency, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant