EmilStenstrom / django-components

Create simple reusable template components in Django.
MIT License
973 stars 62 forks source link

feat: Scoped slots + Updated docs #495

Closed JuroOravec closed 1 week ago

JuroOravec commented 2 weeks ago

~Implementation may still change based on feedback.~ This MR is now ready for review, it adds "scoped slot" feature as per https://github.com/EmilStenstrom/django-components/issues/494#issuecomment-2113363070.

I also moved the README sections around a bit, and added a view missing sections. See the screenshot in https://github.com/EmilStenstrom/django-components/pull/495#discussion_r1598567122 and other comments below.

Closes https://github.com/EmilStenstrom/django-components/issues/494

JuroOravec commented 2 weeks ago

I also added a table of contents to README

JuroOravec commented 1 week ago

@EmilStenstrom

I see no tests with context_behavior="isolated", so I guess that all of them are testing "django"?

No, isolated is still the default. Changing the default involves also changing some tests, as well as documentation. That's why I moved it to a separate issue in https://github.com/EmilStenstrom/django-components/issues/498. And I want to first clean up this one to avoid needlessly complex rebases/merges.

JuroOravec commented 1 week ago

Btw, @EmilStenstrom, what do you think of renaming the kwarg through which we access the data from slot_data to simply data?

When we take also https://github.com/EmilStenstrom/django-components/issues/502 into consideration, then IMO the shorter names would make for a cleaner API.

Consider:

{% fill "my_slot" data="slot_data" default="slot_default" %}

vs

{% fill "my_slot" slot_data="slot_data" slot_default="slot_default" %}
EmilStenstrom commented 1 week ago

@JuroOravec Definitely like the data variable better.

JuroOravec commented 1 week ago

@EmilStenstrom Thanks again for your time and for all the feedback! I've changed slot_data to just data, and I'll go ahead and merge this.