diff --git a/changes/224.bugfix.rst b/changes/224.bugfix.rst new file mode 100644 index 0000000..0ddcf45 --- /dev/null +++ b/changes/224.bugfix.rst @@ -0,0 +1 @@ +Assigning a new style object to a node that already has an applicator assigned now properly maintains an association between the applicator and the new style, and triggers a style reapplication. diff --git a/changes/224.misc.rst b/changes/224.misc.rst new file mode 100644 index 0000000..ef86edb --- /dev/null +++ b/changes/224.misc.rst @@ -0,0 +1 @@ +An applicator, once assigned to a node, now receives a reference to its node (as self.node). diff --git a/changes/224.removal.1.rst b/changes/224.removal.1.rst new file mode 100644 index 0000000..a25da88 --- /dev/null +++ b/changes/224.removal.1.rst @@ -0,0 +1 @@ +Supplying an applicator to BaseStyle.copy() has been deprecated. If you need to manually assign an applicator to a style, do it separately, after the copy. diff --git a/changes/224.removal.2.rst b/changes/224.removal.2.rst new file mode 100644 index 0000000..377527f --- /dev/null +++ b/changes/224.removal.2.rst @@ -0,0 +1 @@ +The mechanisms for assigning styles and applicators to nodes, and applying styles, have been reworked. A node will now attempt to apply its style as soon as it is assigned an applicator; this means you should not assign an applicator to a node until the node is sufficiently initialized to apply its style. To accomodate uses that currently do not follow this order, any exceptions resulting from a failed style application are caught, and a runtime warning is issued. In a future version, this will be an exception. diff --git a/src/travertino/declaration.py b/src/travertino/declaration.py index 4919fba..91791af 100644 --- a/src/travertino/declaration.py +++ b/src/travertino/declaration.py @@ -297,6 +297,22 @@ def _applicator(self): def _applicator(self, value): self._assigned_applicator = value + if value is not None: + try: + self.reapply() + # This is backwards compatibility for Toga, which (at least as of + # 0.4.8), assigns style and applicator before the widget's + # implementation is available. + except Exception: + warn( + "Failed to apply style when assigning applicator, or when " + "assigning a new style once applicator is present. Node should be " + "sufficiently initialized to apply its style before it is assigned " + "an applicator. This will be an exception in a future version.", + RuntimeWarning, + stacklevel=2, + ) + ###################################################################### # Interface that style declarations must define ###################################################################### @@ -326,8 +342,17 @@ def update(self, **styles): def copy(self, applicator=None): "Create a duplicate of this style declaration." dup = self.__class__() - dup._applicator = applicator dup.update(**self) + + if applicator is not None: + warn( + "Providing an applicator to BaseStyle.copy() is deprecated. Set " + "applicator afterward on the returned copy.", + DeprecationWarning, + stacklevel=2, + ) + dup._applicator = applicator + return dup def __getitem__(self, name): diff --git a/src/travertino/node.py b/src/travertino/node.py index 73ab0c9..bed40fb 100644 --- a/src/travertino/node.py +++ b/src/travertino/node.py @@ -1,9 +1,11 @@ class Node: def __init__(self, style, applicator=None, children=None): + # Explicitly set the internal attribute first, since the setter for style will + # access the applicator property. + self._applicator = None + + self.style = style self.applicator = applicator - self.style = style.copy(applicator) - self.intrinsic = self.style.IntrinsicSize() - self.layout = self.style.Box(self) self._parent = None self._root = None @@ -14,6 +16,40 @@ def __init__(self, style, applicator=None, children=None): for child in children: self.add(child) + @property + def style(self): + """The node's style. + + Assigning a style triggers an application of that style if an applicator has + already been assigned. + """ + return self._style + + @style.setter + def style(self, style): + self._style = style.copy() + self.intrinsic = self.style.IntrinsicSize() + self.layout = self.style.Box(self) + + if self.applicator: + self.style._applicator = self.applicator + + @property + def applicator(self): + """This node's applicator, which handles applying the style. + + Assigning an applicator triggers an application of the node's style. + """ + return self._applicator + + @applicator.setter + def applicator(self, applicator): + self._applicator = applicator + self.style._applicator = applicator + + if applicator: + applicator.node = self + @property def root(self): """The root of the tree containing this node. diff --git a/tests/test_choices.py b/tests/test_choices.py index fc40fdf..9eea0f8 100644 --- a/tests/test_choices.py +++ b/tests/test_choices.py @@ -1,38 +1,14 @@ from __future__ import annotations -import sys -from dataclasses import dataclass -from unittest.mock import Mock from warnings import catch_warnings, filterwarnings import pytest +from tests.utils import mock_attr, prep_style_class from travertino.colors import NAMED_COLOR, rgb from travertino.constants import GOLDENROD, NONE, REBECCAPURPLE, TOP from travertino.declaration import BaseStyle, Choices, validated_property -if sys.version_info < (3, 10): - _DATACLASS_KWARGS = {"init": False} -else: - _DATACLASS_KWARGS = {"kw_only": True} - - -def prep_style_class(cls): - """Decorator to apply dataclass and mock a style class's apply method.""" - return mock_apply(dataclass(**_DATACLASS_KWARGS)(cls)) - - -def mock_apply(cls): - """Only mock apply, without applying dataclass.""" - orig_init = cls.__init__ - - def __init__(self, *args, **kwargs): - self.apply = Mock() - orig_init(self, *args, **kwargs) - - cls.__init__ = __init__ - return cls - @prep_style_class class Style(BaseStyle): @@ -56,7 +32,7 @@ class Style(BaseStyle): with catch_warnings(): filterwarnings("ignore", category=DeprecationWarning) - @mock_apply + @mock_attr("apply") class DeprecatedStyle(BaseStyle): pass diff --git a/tests/test_declaration.py b/tests/test_declaration.py index 62f4254..753362a 100644 --- a/tests/test_declaration.py +++ b/tests/test_declaration.py @@ -5,7 +5,7 @@ import pytest -from tests.test_choices import mock_apply, prep_style_class +from tests.utils import mock_attr, prep_style_class from travertino.declaration import ( BaseStyle, Choices, @@ -51,7 +51,7 @@ class Style(BaseStyle): with catch_warnings(): filterwarnings("ignore", category=DeprecationWarning) - @mock_apply + @mock_attr("apply") class DeprecatedStyle(BaseStyle): pass @@ -90,6 +90,12 @@ class Sibling(BaseStyle): pass +@prep_style_class +@mock_attr("reapply") +class MockedReapplyStyle(BaseStyle): + pass + + def test_invalid_style(): with pytest.raises(ValueError): # Define an invalid initial value on a validated property @@ -120,6 +126,15 @@ def test_create_and_copy(StyleClass): assert dup.implicit == VALUE3 +def test_deprecated_copy(): + style = MockedReapplyStyle() + + with pytest.warns(DeprecationWarning): + style_copy = style.copy(applicator=object()) + + style_copy.reapply.assert_called_once() + + @pytest.mark.parametrize("StyleClass", [Style, DeprecatedStyle]) def test_reapply(StyleClass): style = StyleClass(explicit_const=VALUE2, implicit=VALUE3) diff --git a/tests/test_node.py b/tests/test_node.py index b9c2c0f..c281eb1 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,12 +1,36 @@ +from unittest.mock import Mock + import pytest -from travertino.declaration import BaseStyle +from tests.utils import mock_attr, prep_style_class +from travertino.declaration import BaseStyle, Choices, validated_property from travertino.layout import BaseBox, Viewport from travertino.node import Node from travertino.size import BaseIntrinsicSize +@prep_style_class +@mock_attr("reapply") class Style(BaseStyle): + int_prop: int = validated_property(Choices(integer=True)) + + class IntrinsicSize(BaseIntrinsicSize): + pass + + class Box(BaseBox): + pass + + def layout(self, root, viewport): + # A simple layout scheme that allocates twice the viewport size. + root.layout.content_width = viewport.width * 2 + root.layout.content_height = viewport.height * 2 + + +@prep_style_class +class BrokenStyle(BaseStyle): + def reapply(self): + raise AttributeError("Missing attribute, node not ready for style application") + class IntrinsicSize(BaseIntrinsicSize): pass @@ -219,3 +243,131 @@ def test_clear(): assert child.root == child assert node.children == [] + + +def test_create_with_no_applicator(): + style = Style(int_prop=5) + node = Node(style=style) + + # Style copies on assignment. + assert isinstance(node.style, Style) + assert node.style == style + assert node.style is not style + + # Since no applicator has been assigned, style wasn't applied. + node.style.reapply.assert_not_called() + + +def test_create_with_applicator(): + style = Style(int_prop=5) + applicator = Mock() + node = Node(style=style, applicator=applicator) + + # Style copies on assignment. + assert isinstance(node.style, Style) + assert node.style == style + assert node.style is not style + + # Applicator assignment does *not* copy. + assert node.applicator is applicator + # Applicator gets a reference back to its node and to the style. + assert applicator.node is node + assert node.style._applicator is applicator + + # Assigning a non-None applicator should always apply style. + node.style.reapply.assert_called_once() + + +@pytest.mark.parametrize( + "node", + [ + Node(style=Style()), + Node(style=Style(), applicator=Mock()), + ], +) +def test_assign_applicator(node): + node.style.reapply.reset_mock() + + applicator = Mock() + node.applicator = applicator + + # Applicator assignment does *not* copy. + assert node.applicator is applicator + # Applicator gets a reference back to its node and to the style. + assert applicator.node is node + assert node.style._applicator is applicator + + # Assigning a non-None applicator should always apply style. + node.style.reapply.assert_called_once() + + +@pytest.mark.parametrize( + "node", + [ + Node(style=Style()), + Node(style=Style(), applicator=Mock()), + ], +) +def test_assign_applicator_none(node): + node.style.reapply.reset_mock() + + node.applicator = None + assert node.applicator is None + + # Should be updated on style as well + assert node.style._applicator is None + # Assigning None to applicator does not trigger reapply. + node.style.reapply.assert_not_called() + + +def test_assign_style_with_applicator(): + style_1 = Style(int_prop=5) + node = Node(style=style_1, applicator=Mock()) + + node.style.reapply.reset_mock() + style_2 = Style(int_prop=10) + node.style = style_2 + + # Style copies on assignment. + assert isinstance(node.style, Style) + assert node.style == style_2 + assert node.style is not style_2 + + assert node.style != style_1 + + # Since an applicator has already been assigned, assigning style applies the style. + node.style.reapply.assert_called_once() + + +def test_assign_style_with_no_applicator(): + style_1 = Style(int_prop=5) + node = Node(style=style_1) + + node.style.reapply.reset_mock() + style_2 = Style(int_prop=10) + node.style = style_2 + + # Style copies on assignment. + assert isinstance(node.style, Style) + assert node.style == style_2 + assert node.style is not style_2 + + assert node.style != style_1 + + # Since no applicator was present, style should not be applied. + node.style.reapply.assert_not_called() + + +def test_apply_before_node_is_ready(): + style = BrokenStyle() + applicator = Mock() + + with pytest.warns(RuntimeWarning): + node = Node(style=style) + node.applicator = applicator + + with pytest.warns(RuntimeWarning): + node.style = BrokenStyle() + + with pytest.warns(RuntimeWarning): + Node(style=style, applicator=applicator) diff --git a/tests/utils.py b/tests/utils.py new file mode 100644 index 0000000..0649494 --- /dev/null +++ b/tests/utils.py @@ -0,0 +1,29 @@ +import sys +from dataclasses import dataclass +from unittest.mock import Mock + +if sys.version_info < (3, 10): + _DATACLASS_KWARGS = {"init": False} +else: + _DATACLASS_KWARGS = {"kw_only": True} + + +def prep_style_class(cls): + """Decorator to apply dataclass and mock apply.""" + return mock_attr("apply")(dataclass(**_DATACLASS_KWARGS)(cls)) + + +def mock_attr(attr): + """Mock an arbitrary attribute of a class.""" + + def returned_decorator(cls): + orig_init = cls.__init__ + + def __init__(self, *args, **kwargs): + setattr(self, attr, Mock()) + orig_init(self, *args, **kwargs) + + cls.__init__ = __init__ + return cls + + return returned_decorator