Opened 10 months ago

Closed 3 months ago

#35192 closed Cleanup/optimization (needsinfo)

Support injecting custom context into widget templates

Reported by: Oxan van Leeuwen Owned by: Christophe Henry
Component: Forms Version: 5.0
Severity: Normal Keywords:
Cc: David Smith, Oxan van Leeuwen, Christophe Henry Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The context available to widget templates is currently very rigid and does not allow any customization. For example, it's currently almost impossible to add an invalid CSS class to widgets that have errors.

I think my preferred solution would be to insert a call to a form renderer method in the callchain between BoundField.as_widget() and Widget.render(), which can be overridden by a custom renderer. It should receive the BoundField itself as an argument so that it has access to the current state. Something like the following sketch:

class BoundField:
    def as_widget(self, widget=None, attrs=None, only_initial=False):
        ... # leave current code as-is, except for last call to widget.render()
        return self.form.renderer.render_widget(
            field=self,
            widget=widget,
            name=self.html_initial_name if only_initial else self.html_name,
            value=value,
            attrs=attrs,
        )

class BaseRenderer:
    def render_widget(self, field, widget, name, value, attrs=None):
        # this can be overridden to add/change/remove `attrs`, or to render the widget in an entirely different way
        return widget.render(name, value, attrs)

Change History (10)

comment:1 by Mariusz Felisiak, 10 months ago

Cc: David Smith added

comment:2 by David Smith, 10 months ago

Resolution: needsinfo
Status: newclosed

it's currently almost impossible to add an invalid CSS class to widgets that have errors.

I agree that it is currently tricky to add invalid CSS to widgets (<inputs>) and would like to see this eased with Django itself. That was one of my conclusions when I attempted to write a tailwind styled form using just Django itself. See blog post.

I think there's a few different design options here, in addition to your suggestion above I see a couple of additional options.

  • A template filter/tag. This is the approach used by both django-widget-tweaks and django-crispy-forms.
  • Easing use of a custom BoundField. Currently you need to override each field's get_bound_field() to achieve this. Maybe this could also be set on the form renderer? This would also allow folk to make other customisations such as #35191 without adding a long list of options to the renderer.

It's probably worth starting a discussion on the django forum to gain wider views from the community. I'm happy to help with this discussion. As I think we need a wider discussion on the design I'll mark as 'needsinfo' for now.

comment:3 by Oxan van Leeuwen, 10 months ago

I think a custom BoundField could also work indeed. I'm by no means married to my proposal, it was just the most sensible thing I could come up with :)

I agree that it could be valuable to have a wider discussion about the design here, but unfortunately I don't have the bandwidth at the moment to drive such a discussion.

comment:4 by Christophe Henry, 6 months ago

Why was this ticket closed? I think allowing to use a custom BoundField would be a nice enhancement.

comment:5 by Christophe Henry, 6 months ago

Has patch: set

comment:6 by Christophe Henry, 6 months ago

Resolution: needsinfo
Status: closednew

comment:7 by Christophe Henry, 6 months ago

Cc: Christophe Henry added
Owner: changed from nobody to Christophe Henry
Status: newassigned

in reply to:  4 comment:8 by Sarah Boyce, 6 months ago

Resolution: needsinfo
Status: assignedclosed

Replying to Christophe Henry:

Why was this ticket closed? I think allowing to use a custom BoundField would be a nice enhancement.

This ticket wanted more community input before progressing.

But good news is recently a similar ticket #35521 was raised which lead to this forum discussion and your PR was referenced.
Can you add some of your thoughts into that discussion and see if we can come to agreement as to what would be the best approach for this?

I will close this ticket while the discussion is in place (this ticket might not best represent what we decide to progress in the end) but we can reopen this later if needed 👍

comment:9 by Matthias Kestenholz, 3 months ago

Resolution: needsinfo
Status: closednew
Triage Stage: UnreviewedAccepted

We have a somewhat active discussion on the forum.

The proposal to add field_css_class is probably dead in the waters. Customizing the BoundField class is a better proposal since it opens the door for many customization possibilities in third party projects and websites, even though it's slightly more complex than #35521.

I'm therefore reopening this ticket and have reviewed the PR somewhat. If I get some blowback here then that's my fault.

in reply to:  9 comment:10 by Natalia Bidart, 3 months ago

Resolution: needsinfo
Status: newclosed
Triage Stage: AcceptedUnreviewed

Replying to Matthias Kestenholz:

We have a somewhat active discussion on the forum.

Thank you Matthias for the update!

While I agree that the forum post is progressing nicely towards what it looks a decent consensus, I think it would be wise to wait a few more posts before re-opening this ticket. I'll add a comment in the thread with my thoughts and add a reminder to myself to re-check in about a week.

Note: See TracTickets for help on using tickets.
Back to Top