-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 validation 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. | ||
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is rather sad, forcing the test to work, but I suppose it checks that changing
ui.savebuttonbar.unsaved_changes
andui.is_valid.value
have the intended effects on the save button.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.
Yeah, this was not my first choice. I tried calling the handlers that I thought
ipyautoui
was invoking, but i) they were private (started with_
) and ii) didn't do anything.