Code

Opened 3 years ago

Closed 20 months ago

#15244 closed New feature (wontfix)

Documentation needed for RadioFieldRenderer

Reported by: SmileyChris Owned by: lkeijser
Component: Documentation Version: master
Severity: Normal Keywords: renderer widget
Cc: mmitar@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Overriding the RadioSelect renderer is possible, but not documented.

Attachments (1)

doc-custom_renderer_howto.patch (2.1 KB) - added by lkeijser 3 years ago.
patch to include custom renderer documentation

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by SmileyChris

  • Component changed from Uncategorized to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 3 years ago by lkeijser

patch to include custom renderer documentation

comment:2 Changed 3 years ago by lkeijser

  • Keywords renderer widget added
  • Owner changed from nobody to lkeijser
  • Status changed from new to assigned
  • Triage Stage changed from Accepted to Ready for checkin
  • Version set to SVN

I've created a patch against the latest trunk (rev. 15487). As this is my first patch, format might not be correct so please review.

comment:3 Changed 3 years ago by gabrielhurley

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

First off, thank you for the patch.

Second, in the future please don't mark your own patch as "Ready For Checkin"--when you upload a patch you should mark the "has patch" checkbox, and then once it has been peer-reviewed the reviewer will either provide feedback on the ticket or decide it looks good and mark it as RFC.

Before I get to the patch itself, I'd like to point out that the existence of RadioFieldRenderer is inconsistent in and of itself. All the other widgets contain the rendering logic inside their own render methods, and you would customize them at that point. Even the other "special case" input type--select--doesn't get a special "renderer" class. I see why the renderer class is useful, it's just inconsistent.

As for the patch itself:

  1. I would rather see this information added to the widgets reference documentation instead of creating a new how to page. I've always been surprised that there wasn't a topic guide on widgets, but that's the subject of a different ticket. Right now, when people want to know about widgets, they go to ref/forms/widgets and there's already information about "customizing widgets" there... so let's keep the information together.
  1. Because RadioFieldRenderer is a completely one-off solution, more care should be taken to note that this does not apply to all widgets, just this one.
  1. The force_unicode(w) for w in self on line 21 ought to be explained. I know what it does because I've read the code for RadioFieldRenderer and know that it has a custom __iter__ method which yields RadioInput objects, but that's not readily obvious to anyone else.
  1. The new documentation about customizing the radio widget should be linked from the RadioSelect widget docs.
  1. Your first example imports mark_safe and StrAndUnicode which are not used.
  1. On a purely grammatical note, "In particular its renderer function." [lines 9-10 of your patch] is a sentence fragment.
  1. Not that there's anything technically wrong with is, but I might pick a field name other than os for the example simply because os is a very common python module and there's no reason to trigger confusion in people's minds.

This patch is a good start, and with a little more work will make a valuable addition to the docs. With a quick turnaround it can even still make it into the 1.3 milestone.

comment:4 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 2 years ago by mitar

  • Cc mmitar@… added

comment:8 Changed 20 months ago by timo

I started revising this patch per Russ's feedback, but I noticed Django 1.4 allows looping over the radio choices in the template. This seems like a much easier solution to the problem this patch attempts to solve. Should we still include this?

comment:9 Changed 20 months ago by timo

  • Resolution set to wontfix
  • Status changed from assigned to closed

And upon further reading (#15667), I see this API is being deprecated, closing this as won't fix in light of that.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.