-
Notifications
You must be signed in to change notification settings - Fork 12
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
Connect state of save/revert buttons to something sensible #310
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
from stellarphot.settings import Camera, ui_generator | ||
from stellarphot.settings.tests.test_models import TEST_CAMERA_VALUES | ||
|
||
|
||
class TestUiGenerator: | ||
def test_camera(self): | ||
ui = ui_generator(Camera) | ||
# The description should be the beginning of the docstring | ||
assert Camera.__doc__.strip().startswith(ui.description.split()[0]) | ||
|
||
# We always want to show nullable fields | ||
assert ui.show_null | ||
|
||
# Which means we don't need the button to show/hide them | ||
assert ui.bn_shownull.layout.display == "none" | ||
|
||
# For now we do not show the validaation output because it is painfully | ||
# verbose and not very helpful. | ||
assert not ui.show_validation | ||
|
||
# We always display nested models | ||
assert ui.open_nested | ||
|
||
# Finally, we don't want the widget to update continuously because it | ||
# will overwrite the value the user entered. | ||
for widget in ui.di_widgets.values(): | ||
if hasattr(widget, "continuous_update"): | ||
assert not widget.continuous_update | ||
|
||
def test_disabled_state_save_revert_button(self): | ||
# We want the save and revert buttons to be enabled only when | ||
# 1. The user has made a change, AND | ||
# 2. The value in the widget is a valid pydantic model | ||
|
||
ui = ui_generator(Camera) | ||
# The save button should be disabled | ||
assert ui.savebuttonbar.bn_save.disabled | ||
assert ui.savebuttonbar.bn_revert.disabled | ||
|
||
# Set one field to a valid value.... | ||
ui.value["name"] = "test" | ||
|
||
# ...and the save button should still be disabled | ||
assert ui.savebuttonbar.bn_save.disabled | ||
assert ui.savebuttonbar.bn_revert.disabled | ||
|
||
# Set a valid value | ||
ui.value = TEST_CAMERA_VALUES | ||
|
||
# So it turns out that the validation stuff only updates when changes are made | ||
# in the UI rather than programmatically. Since we know we've set a valid value, | ||
# and that we've made changes we just manually set the relevant values. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is rather sad, forcing the test to work, but I suppose it checks that changing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this was not my first choice. I tried calling the handlers that I thought |
||
ui.savebuttonbar.unsaved_changes = True | ||
ui.is_valid.value = True | ||
|
||
# The save button should now be enabled | ||
assert not ui.savebuttonbar.bn_save.disabled | ||
assert not ui.savebuttonbar.bn_revert.disabled | ||
|
||
# Click on save | ||
ui.savebuttonbar.bn_save.click() | ||
|
||
# Unsaved changes should be False | ||
assert not ui.savebuttonbar.unsaved_changes | ||
|
||
# The save and revert buttons should now be disabled | ||
assert ui.savebuttonbar.bn_save.disabled | ||
assert ui.savebuttonbar.bn_revert.disabled |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,9 @@ def ui_generator(model): | |
# to use the entire docstring, which is often too long. | ||
ui.description = _extract_short_description(model.__doc__) | ||
|
||
# Always show nested models | ||
ui.open_nested = True | ||
|
||
# Validation is checked every time a value is changed, and the contents of each | ||
# field are written to the widget if validation passes. In at least one case, | ||
# the "Observatory" model, this is not helpful because the format of lat/lon is | ||
|
@@ -43,4 +46,34 @@ def ui_generator(model): | |
if hasattr(widget, "continuous_update"): | ||
widget.continuous_update = False | ||
|
||
# The save and revert buttons should be enabled only when the user has made a | ||
# change AND the value in the widget is a valid pydantic model. | ||
# We begin by disabling the buttons. | ||
ui.savebuttonbar.bn_save.disabled = True | ||
ui.savebuttonbar.bn_revert.disabled = True | ||
|
||
# Now we add observers to enable/disable the buttons based on the validity of | ||
# the value and whether there are unsaved changes. | ||
for button in [ui.savebuttonbar.bn_save, ui.savebuttonbar.bn_revert]: | ||
ui.is_valid.observe(_handle_save_revert_button_state(ui, button), "value") | ||
ui.savebuttonbar.observe( | ||
_handle_save_revert_button_state(ui, button), "unsaved_changes" | ||
) | ||
|
||
return ui | ||
|
||
|
||
def _handle_save_revert_button_state(widget, button): | ||
""" | ||
Return a callback that will enable/disable the save and revert buttons based | ||
on the validity of the value and whether there are unsaved changes. | ||
""" | ||
|
||
def handler(_): | ||
""" | ||
A handler must take an argument but we don't use it here. | ||
""" | ||
needs_to_save = widget.is_valid and widget.savebuttonbar.unsaved_changes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clever bit of logic here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to break it into steps because I was confusing myself when I tried to make it a single line 😬 |
||
button.disabled = not needs_to_save | ||
|
||
return handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo of "validation", somewhat ironic since it passed initial review by the author....