-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
Fixed #35535 -- Add simple block tag #18343
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
Conversation
d710f1a
to
b75ca05
Compare
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.
Thank you for this @RealOrangeOne, added initial comments
We need a release note in 5.2 👍
b75ca05
to
8f4f47a
Compare
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.
Thank you for the updates @RealOrangeOne 👍
We still need a release note adding to 5.2
8f4f47a
to
46bfe24
Compare
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.
Added some comments, thank you @RealOrangeOne for this work!
b179626
to
8f33d52
Compare
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.
Thank you @RealOrangeOne! I gave this another top-bottom pass. Once the minor nitpicks are fixed, I need to do a in-depth review in terms of test coverage and functionality. That would be after DjangoCon US.
Amazing work so far! 🌟
8f33d52
to
e171598
Compare
8e211e0
to
cc67d6e
Compare
cc67d6e
to
c4b2855
Compare
c4b2855
to
53e9444
Compare
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.
I have added a new subsection in the docs with a more complete/complex example. I built it from real use case I had to solve a while ago. From my view, this is ready to be merged.
@sarahboyce @adamchainz @carltongibson I'm keen to know what you think of the latest version of the docs. Ideally I'll merge in about a week, depending on your feedback. Thanks!
53e9444
to
16fe320
Compare
Co-authored-by: Natalia <[email protected]>
16fe320
to
7a30f72
Compare
Trac ticket number
ticket-35535
See also related forum thread: https://forum.djangoproject.com/t/feature-proposal-simple-block-tag/32229
Branch description
Django's
simple_tag
is difficult to work with if you need to capture more complex content as one of the arguments.This PR adds the ability to define custom "block" tags, which capture their contents as the
content
argument.There are probably some built-in tags which could take advantage of this, but I've left those out-of-scope of this PR. The same is true for extra niceties like a built-in
capture
tag, passing the un-rendered content into the tag instead or a combination withinclusion_tag
etc. See #18343 (comment) for the small diff needed to supportinclusion_block_tag
in future.Checklist
main
branch.