On 10/27/06, Adrian Holovaty <[EMAIL PROTECTED]> wrote:
> It's not 100% finished. Not all the common validators are implemented,
> nor are all of the HTML form widgets. But the basic design is there.
> I'm trying to "release" early and often to get plenty of critiques
> before things get stable.

Looks pretty good so far. Some comments below.

> * Each Field has a 'widget' attribute, which is set to the Widget
> class to use when rendering it. This is nice and decoupled but, more
> importantly, practical.

+1 on this. I like the separation of widget from field; it allows easy
wrapping of the basic widgets, but also provides a mechanism by which
different widgets can share to_python implementations (e.g., different
color selection widgets)

However, I'm not so keen on the combination of Field.widget with the
hardcoded as_text, as_textarea approach suggested by the commented
sections of BoundField. I would suggest that these methods are
describing which widget should be used, and as such, should be dynamic
based on the widgets available on a Field:
- Field should hold a dictionary of the widgets that can be used for
display, and the name of the default widget.
- BoundField should implement __getitem__, with the value providing
the key into the widget dictionary.
- __str__ on BoundField returns the default widget from the Field
widget dictionary

This means each Field can define the widget types that are useful and
expose them through the template. The constructor for a Field might
also be extended to allow users to easily add their own widgets to the
available list.

I would also suggest that the coupling between to_python and the POST
dictionary be loosened. Specifically:

- Add an extract method be provided on the Widget to describe how to
pull POST data for the Widget out of the POST dictionary.

def extract(self, name, data):
    return { 'value': data.get(name,None) }

- Modify the call to field.to_python to use extract():

    value = field.to_python(**widget.extract(name, post_data)])

Why? So that you can implement widgets that use multiple input fields
to produce a single output value. For example:
  - specifying color as three separate R,G,B text fields/sliders, but
stored as "#RRGGBB" in the database
  - a latitude/longitude entered as degrees, minutes and seconds but
stored as a float in radians in the database.

Using the color example, extract would be something like:

def extract(self, name, data):
    return {
        'red': data.get(name + '_red', None),
        'green': data.get(name + '_green', None),
        'blue': data.get(name + '_blue', None)
    }

and to_python becomes:

def to_python(self, red, green, blue):
   return (...calculated value...)

However, this means that to_python must be smart enough to use the
provided kwargs to identify what type of data has been provided. If
two different widgets on a field extract different data, to_python has
to be extra smart. To overcome this, let the widget define the
to_python implementation that is compatible with the widget:

class RGBSliderWidget(Widget):
    to_python = 'to_python_from_triple'
    def extract() ...
    def render() ...

class TextWidget(Widget):
    to_python = 'to_python'
    def extract() ...
    def render() ...

class ColorField(Field):
    widgets = {
        'rgbslider' : RGBSliderWidget,
        'text': TextWidget
    }
    default_widget = 'text'

    def to_python(self, value):
        ...
    def to_python_from_triple(self, red, green, blue)
        ...

and use getattr to dynamically load the to_python implementation based
on the widget requested.

> I'm not sure what the ideal behavior is here. The (at least) three
> possibilities are:
>
>    - Always render the original input data, even if the field was
> valid and got converted to something slightly different by
> to_python().

+1 to this option for me. The BoundField is bound to raw data. It
should display that raw data. Especially in the case of __str__, since
it is providing the default widget rendering for templates.

The other two options smell like magic. You will never know whether
you have converted data or not; predictability goes right out the
window.

> * There's a to-do list at the top. As I said, this is just a work in
> progress at this point. I welcome any thoughts, comments and patches!

One management issue that concerns me: Is there a plan for getting
this into trunk without messing with installations that are currently
using trunk? This is a biggish API change if it overlaps the existing
django.forms. Were you planning on tagging a 0.96 (or a 0.95.1)
release to capture bugfixes in the post-0.95 trunk?

Yours,
Russ Magee %-)

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers
-~----------~----~----~----~------~----~------~--~---

Reply via email to