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

Refactor title decoration #373

Merged
merged 3 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 63 additions & 39 deletions stellarphot/settings/custom_widgets.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
# Some settings require custom widgets to be displayed in the GUI. These are defined in
# this module.

# This workaround is for Python < 3.11. It is not needed in Python 3.11 and later.
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't we just require 3.11? I think we opted to require 3.10 last summer, but at this point... In any case, if you are wedded to supporting 3.10, keep this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could require 3.11. I wasn't aware until I opened this PR that StrEnum was new in 3.11.

try:
from enum import StrEnum
except ImportError:
from enum import Enum

class StrEnum(str, Enum):
"""
A class that allows for string values in an Enum pre-Python 3.11.
"""


import ipywidgets as ipw
import traitlets as tr
from ipyautoui.autoobject import AutoObject
Expand All @@ -14,7 +26,7 @@
ui_generator,
)

__all__ = ["ChooseOrMakeNew", "Confirm"]
__all__ = ["ChooseOrMakeNew", "Confirm", "SettingWithTitle"]

DEFAULT_BUTTON_WIDTH = "300px"

Expand Down Expand Up @@ -643,9 +655,6 @@ class SettingWithTitle(ipw.VBox):
The setting widget to be displayed.
"""

SETTING_NOT_SAVED = "❗️"
SETTING_IS_SAVED = "✅"

def __init__(self, plain_title, widget, header_level=2, *args, **kwargs):
super().__init__(*args, **kwargs)
self._header_level = header_level
Expand All @@ -655,47 +664,62 @@ def __init__(self, plain_title, widget, header_level=2, *args, **kwargs):
self.children = [self.title, self._widget]
# Set up an observer to update title decoration when the settings
# change.
self._widget.observe(self.decorate_title, names="_value")
self._widget.observe(self._title_observer, names="_value")
# Also update after the save button is clicked
self._widget.savebuttonbar.fns_onsave_add_action(self.decorate_title)
self._widget.savebuttonbar.fns_onsave_add_action(self._title_observer)

def _title_observer(self, change=None):
"""
Observer for the title of the widget.
"""
self.title.value = self._format_title(
title_decorator(self._plain_title, self._widget, change)
)

def _format_title(self, decorated_title):
return f"<h{self._header_level}>{decorated_title}</h{self._header_level}>"

def decorate_title(self, change=None):
"""
Decorate the title based on whether the settings need to be saved.

Parameters
----------
class SaveStatus(StrEnum):
"""
Class to define the symbols used to represent a save status.
"""

change : dict, optional
Change dictionary from a traitlets event. It is optional so that this
method can be called without a change dictionary.
"""
# Keep track of settings state -- if dirty is true, the settings
# need to be saved.
SETTING_NOT_SAVED = "❗️"
SETTING_IS_SAVED = "✅"
SETTING_SHOULD_BE_REVIEWED = "🔆"


def title_decorator(plain_title, autoui_widget, change=None):
"""
Decorate the title based on whether the settings need to be saved.

Parameters
----------

change : dict, optional
Change dictionary from a traitlets event. It is optional so that this
method can be called without a change dictionary.
"""
# Keep track of settings state -- if dirty is true, the settings
# need to be saved.
dirty = False

# If we got here via a traitlets event then change is a dict with keys
# "new" and "old", check that case first. By checking explicitly for
# this case, we guarantee that we catch changes in value even if
# the button bar's unsaved_changes is still False.
try:
if change["new"] != change["old"]:
dirty = True
except (KeyError, TypeError):
dirty = False

# If we got here via a traitlets event then change is a dict with keys
# "new" and "old", check that case first. By checking explicitly for
# this case, we guarantee that we catch changes in value even if
# the button bar's unsaved_changes is still False.
try:
if change["new"] != change["old"]:
dirty = True
except (KeyError, TypeError):
dirty = False

# The unsaved_changes attribute is not a traitlet, and it isn't clear when
# in the event handling it gets set. When not called from an event, though,
# this function can only used unsaved_changes to decide what the title
# should be.
if self._widget.savebuttonbar.unsaved_changes or dirty:
self.title.value = self._format_title(
f"{self._plain_title} {self.SETTING_NOT_SAVED}"
)
else:
self.title.value = self._format_title(
f"{self._plain_title} {self.SETTING_IS_SAVED}"
)
# The unsaved_changes attribute is not a traitlet, and it isn't clear when
# in the event handling it gets set. When not called from an event, though,
# this function can only used unsaved_changes to decide what the title
# should be.
if autoui_widget.savebuttonbar.unsaved_changes or dirty:
return f"{plain_title} {SaveStatus.SETTING_NOT_SAVED}"
else:
return f"{plain_title} {SaveStatus.SETTING_IS_SAVED}"
17 changes: 9 additions & 8 deletions stellarphot/settings/tests/test_custom_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from stellarphot.settings.custom_widgets import (
ChooseOrMakeNew,
Confirm,
SaveStatus,
SettingWithTitle,
)
from stellarphot.settings.tests.test_models import (
Expand Down Expand Up @@ -827,32 +828,32 @@ def test_title_decoration(self):
camera_title = SettingWithTitle(plain_title, camera)

# At the moment the title should not be decorated
assert camera_title.SETTING_IS_SAVED not in camera_title.title.value
assert camera_title.SETTING_NOT_SAVED not in camera_title.title.value
assert SaveStatus.SETTING_IS_SAVED not in camera_title.title.value
assert SaveStatus.SETTING_NOT_SAVED not in camera_title.title.value

# There should also be no unsaved changes at the moment
assert not camera.savebuttonbar.unsaved_changes

# Manually set unsaved_changes then call the change handler, which should
# add an indication that there are unsaved changes.
camera.savebuttonbar.unsaved_changes = True
camera_title.decorate_title()
assert camera_title.SETTING_NOT_SAVED in camera_title.title.value
camera_title._title_observer()
assert SaveStatus.SETTING_NOT_SAVED in camera_title.title.value

# Go back to unsaved_changes being False
camera.savebuttonbar.unsaved_changes = False

# Manually call the change handler, which should add an indication that
# saves have been done.
camera_title.decorate_title()
assert camera_title.SETTING_IS_SAVED in camera_title.title.value
camera_title._title_observer()
assert SaveStatus.SETTING_IS_SAVED in camera_title.title.value

# Now change the camera value, which should trigger the change handler
camera.value = TEST_CAMERA_VALUES

assert camera_title.SETTING_NOT_SAVED in camera_title.title.value
assert SaveStatus.SETTING_NOT_SAVED in camera_title.title.value

# Finally, click the save button and the title should be decorated with
# the saved indication.
camera.savebuttonbar.bn_save.click()
assert camera_title.SETTING_IS_SAVED in camera_title.title.value
assert SaveStatus.SETTING_IS_SAVED in camera_title.title.value