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

Parent context refactor #24

Merged
merged 8 commits into from
Mar 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@
**Checklist**
<!-- For each of the following: check `[x]` if fulfilled or mark as irrelevant `[-]` if not applicable. -->
- [ ] [CHANGELOG.md](https://github.com/tbrlpld/laces/blob/main/CHANGELOG.md) has been updated.
- [ ] [README.md](https://github.com/tbrlpld/laces/blob/main/README.md) has been updated.
- [ ] Checked compatibility with Wagtail.
- [ ] Self code reviewed.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- ...
- Refactored handling of `parent_context` between `render_html` and `get_context_data`. This change is Wagtail-compatible. ([#24](https://github.com/tbrlpld/laces/pull/24))

### Removed

Expand Down
32 changes: 17 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ from laces.components import Component


class WelcomePanel(Component):
def render_html(self, parent_context):
def render_html(self, parent_context=None):
return format_html("<h1>Hello World!</h1>")
```

Expand Down Expand Up @@ -156,7 +156,7 @@ from laces.components import Component
class WelcomePanel(Component):
template_name = "my_app/components/welcome.html"

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return {"name": "Alice"}
```

Expand Down Expand Up @@ -189,7 +189,7 @@ class WelcomePanel(Component):
def __init__(self, name):
self.name = name

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return {"name": self.name}
```

Expand Down Expand Up @@ -222,9 +222,9 @@ You may have noticed in the above examples that the `render_html` and `get_conte
This is the context of the template that is rendering the component.
The `parent_context` is passed into the `render_html` method by the `{% component %}` template tag.

In the default implementation of the `render_html` method, the `parent_context` is passed to the `get_context_data` method.
The `render_html` method, then, passes the `parent_context` to the `get_context_data` method.
The default implementation of the `get_context_data` method, however, ignores the `parent_context` argument and returns an empty dictionary.
To make use of it, you will have to override the `get_context_data` method.
To make use of it, you can override the `get_context_data` method.

Relying on data from the parent context somewhat forgoes some of the benefits of components, which is tying the data and template together.
Especially for nested uses of components, you now require that the data in the right format is passed through all layers of templates again.
Expand All @@ -242,7 +242,9 @@ from laces.components import Component
class WelcomePanel(Component):
template_name = "my_app/components/welcome.html"

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
if not parent_context or "request" not in parent_context:
return {}
return {"name": parent_context["request"].user.first_name}
```

Expand Down Expand Up @@ -308,7 +310,7 @@ To limit the parent context variables passed to the component to only those vari
{% component welcome with name=request.user.first_name only %}
```

**Note**: Both, `with` and `only`, only affect the `parent_context` which is passed to the component's `render_html` and `get_context_data` methods. They do not have any direct effect on actual context that is passed to the component's template. E.g. if the component's `get_context_data` method returns a dictionary which always contains a key `foo`, then that key will be available in the component's template, regardless of whether `only` was used or not.
**Note**: Both, `with` and `only`, affect the `parent_context` which is passed to the component's `render_html` and `get_context_data` methods. They do not have any direct effect on actual context that is passed to the component's template. E.g. if the component's `get_context_data` method returns a dictionary which always contains a key `foo`, then that key will be available in the component's template, regardless of whether `only` was used or not.

#### Store the rendered output in a variable with `as`

Expand Down Expand Up @@ -476,7 +478,7 @@ class Dashboard(Component):
self.welcome = WelcomePanel(name=user.first_name)
...

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return {"welcome": self.welcome}
```

Expand Down Expand Up @@ -534,7 +536,7 @@ class Dashboard(Component):
]
...

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return {"panels": self.panels}
```

Expand Down Expand Up @@ -572,7 +574,7 @@ class Section(Component):
self.children = children
...

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return {"children": self.children}


Expand Down Expand Up @@ -663,12 +665,12 @@ class WelcomePanel(Component):

name: str

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return asdict(self)
```

With dataclasses we define the name and type of the properties we want to pass to the component in the class definition.
Then, we can use the `asdict` function to convert the dataclass instance to a dictionary that can be directly as the template context.
Then, we can use the `asdict` function from the `dataclasses` module to convert the dataclass instance to a dictionary that can be directly used as the template context.

The `asdict` function only adds keys to the dictionary that were defined as the properties defined in the dataclass.
In the above example, the dictionary returned by `asdict` would only contain the `name` key.
Expand Down Expand Up @@ -713,7 +715,7 @@ class WelcomePanel(Component):
profile_image_url=user.profile.image.url,
)

def get_context_data(self, parent_context):
def get_context_data(self, parent_context=None):
return asdict(self)
```

Expand All @@ -736,9 +738,9 @@ def home(request):
)
```

The constructor method allows us to keep our view very simple and clean as all the data preparation is encapsulated in the component.
The constructor method allows us to keep our view very simple and clean, as all the data preparation is encapsulated in the component.

As in the example above, custom constructor methods pair very well with the use of dataclasses, but they can of course also be used without them.
As in the example above, custom constructor methods pair very well with the use of dataclasses, but they can also be used without them.

## About Laces

Expand Down
6 changes: 2 additions & 4 deletions laces/components.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from typing import TYPE_CHECKING, List

from django.forms.widgets import Media, MediaDefiningClass
from django.template import Context
from django.template.loader import get_template

from laces.typing import HasMediaProperty
Expand Down Expand Up @@ -45,8 +44,6 @@ def render_html(
consisting of HTML should typically be returned as a
`django.utils.safestring.SafeString` instance.
"""
if parent_context is None:
parent_context = Context()
context_data = self.get_context_data(parent_context)
if context_data is None:
raise TypeError("Expected a dict from get_context_data, got None")
Expand All @@ -56,8 +53,9 @@ def render_html(

def get_context_data(
self,
parent_context: "RenderContext",
parent_context: "Optional[RenderContext]" = None,
) -> "Optional[RenderContext]":
"""Return the context data to render this component with."""
return {}

# fmt: off
Expand Down
15 changes: 14 additions & 1 deletion laces/test/example/components.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with fixed `name`."""
return {"name": "Alice"}


Expand All @@ -54,6 +55,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with `name` attribute."""
return {"name": self.name}


Expand All @@ -68,6 +70,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with `self`."""
return {"self": self}


Expand All @@ -81,13 +84,20 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with dataclass object as dict."""
return asdict(self)


class PassesNameFromParentContextComponent(Component):
template_name = "components/hello-name.html"

def get_context_data(self, parent_context: "RenderContext") -> "RenderContext":
def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return the `name` from the parent context as the only key in the data."""
if not parent_context or "name" not in parent_context:
return {}
return {"name": parent_context["name"]}


Expand All @@ -103,6 +113,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with heading and content."""
return {
"heading": self.heading,
"content": self.content,
Expand All @@ -121,6 +132,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with heading and items."""
return {
"heading": self.heading,
"items": self.items,
Expand Down Expand Up @@ -170,6 +182,7 @@ def get_context_data(
self,
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return context data with fixed `name`."""
return {"name": "Media"}

class Media:
Expand Down
14 changes: 13 additions & 1 deletion laces/test/tests/test_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,24 @@ def test_template_name(self) -> None:
"components/hello-name.html",
)

def test_get_context_data(self) -> None:
def test_get_context_data_with_name_in_parent_context(self) -> None:
self.assertEqual(
self.component.get_context_data(parent_context={"name": "Dan"}),
{"name": "Dan"},
)

def test_get_context_data_without_name_in_parent_context(self) -> None:
self.assertEqual(
self.component.get_context_data(parent_context={"notname": "Dan"}),
{},
)

def test_get_context_data_without_parent_context(self) -> None:
self.assertEqual(
self.component.get_context_data(),
{},
)

def test_render_html(self) -> None:
self.assertEqual(
self.component.render_html(parent_context={"name": "Dan"}),
Expand Down
26 changes: 21 additions & 5 deletions laces/tests/test_components.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@


if TYPE_CHECKING:
from typing import Any, Optional, Union
from typing import Optional

from laces.typing import RenderContext

Expand All @@ -34,9 +34,9 @@ def test_render_html(self) -> None:
with self.assertRaises(AttributeError):
self.component.render_html()

def test_get_context_data_parent_context_empty_context(self) -> None:
def test_get_context_data_with_parent_context_empty_context(self) -> None:
"""
Test the default get_context_data.
Test the default `get_context_data`.

The parent context should not matter, but we use it as it is used in
`render_html` (which passes a `Context` object).
Expand All @@ -46,6 +46,20 @@ def test_get_context_data_parent_context_empty_context(self) -> None:
self.assertIsInstance(result, dict)
self.assertEqual(result, {})

def test_get_context_data_with_parent_context_none(self) -> None:
"""Test the default `get_context_data` when received `parent_context=None`."""
result = self.component.get_context_data(parent_context=None)

self.assertIsInstance(result, dict)
self.assertEqual(result, {})

def test_get_context_data_without_parent_context_argument(self) -> None:
"""Test the default `get_context_data` when not passing `parent_context`."""
result = self.component.get_context_data()

self.assertIsInstance(result, dict)
self.assertEqual(result, {})

def test_media(self) -> None:
"""
Test the `media` property.
Expand Down Expand Up @@ -125,8 +139,9 @@ class ExampleComponent(Component):

def get_context_data(
self,
parent_context: "Optional[RenderContext]",
parent_context: "Optional[RenderContext]" = None,
) -> "RenderContext":
"""Return a context dict with fixed content."""
return {"name": "World"}

# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -154,8 +169,9 @@ def test_render_html_when_get_context_data_returns_None(self) -> None:
class ExampleComponent(Component):
def get_context_data(
self,
parent_context: "Optional[Union[Context, dict[str, Any]]]",
parent_context: "Optional[RenderContext]" = None,
) -> None:
"""Return `None` as the context data."""
return None

# -----------------------------------------------------------------------------
Expand Down
Loading