Skip to content

Fix CheckBox default state validation and initialization #553

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

Merged
merged 1 commit into from
May 5, 2023

Conversation

penguinolog
Copy link
Collaborator

Fix #552

Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

@@ -257,15 +270,12 @@ def set_state(self, state: bool | Literal['mixed'], do_callback: bool = True) ->
# self._state is None is a special case when the CheckBox
# has just been created
old_state = self._state
if do_callback and old_state is not None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old_state is not None beacause explicit initialization used


# Initial create expect no callbacks call, create explicit
super().__init__(
Columns(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit full init -> no "semi-initialized state"

"""
super().__init__(None) # self.w set by set_state below
self._label = Text("")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can set all in 1 call instead of chain "construct empty -> call helper to call modifier to replace value from constructor"

[...'[X] Extra onions ']
>>> cb.render((20,), focus=True).text
[b'[X] Extra onions ']
>>> CheckBox("Test", None)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for issue

@penguinolog penguinolog requested a review from wardi May 5, 2023 11:03
@penguinolog penguinolog merged commit d26cb42 into urwid:master May 5, 2023
@penguinolog penguinolog deleted the checkbox_init branch May 5, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox created with None state left incompletely initialized
2 participants