newforms error design

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

newforms error design

alex.v.koval@gmail.com

Hi!

Today I tried to use django.newforms, and the first thing which I stuck
upon was the design of errors. Since we are using tables for our forms,
it seems to be hardcoded to use separate row to display errors. Not
sure how nicely to display errors - all I've achieved so far is that
varaint:

http://www.halogen-dg.com/alex/django/form_error_design2.gif

But in fact it does not look user intuitive. Before newforms I've used
non-official contrib.formgenerator package, and now, since it was not
incorporated to the django main code, I am porting my code to newforms.


The problem is visual. I do not like to present end users so ugly
looking errors. As temp. solution I have patched as_table() output, so
it takes optional argument:

errors_on_separate_row

So, now, when I call it from my application, I can specify to display
errors on same row, what looks
the following way (much more intuitive, IMO):

http://www.halogen-dg.com/alex/django/form_error_design1.gif

As conclusion, I would say, that current  'def _html_output(' is not
the best out here. And the more
people would like to customize it, the more difficult to read code will
became. IMO, best solution
would be using Form and Field template here. So, end user can override
this template into
his project and make any kind of formatting of output.

Here is my patch:

alex@dom ~/D/django_src/django/newforms $ svn diff
Index: forms.py
===================================================================
--- forms.py    (revision 4234)
+++ forms.py    (working copy)
@@ -97,9 +97,14 @@
                     top_errors.extend(['(Hidden field %s) %s' % (name,
e) for e in bf_errors])
                 hidden_fields.append(unicode(bf))
             else:
-                if errors_on_separate_row and bf_errors:
-                    output.append(error_row % bf_errors)
-                output.append(normal_row % {'errors': bf_errors,
'label': bf.label_tag(escape(bf.label+':')), 'field': bf})
+                if bf_errors:
+                    if errors_on_separate_row:
+                        output.append(error_row % bf_errors)
+                        output.append(normal_row % {'label':
bf.label_tag(escape(bf.label+':')), 'field': bf})
+                    else:
+                        output.append(error_row % {'error': bf_errors,
'label': bf.label_tag(escape(bf.label+':')), 'field': bf})
+                else:
+                    output.append(normal_row % {'label':
bf.label_tag(escape(bf.label+':')), 'field': bf})
         if top_errors:
             output.insert(0, error_row % top_errors)
         if hidden_fields: # Insert any hidden fields in the last row.
@@ -112,9 +117,14 @@
                 output.append(str_hidden)
         return u'\n'.join(output)

-    def as_table(self):
+    def as_table(self,errors_on_separate_row=True):
         "Returns this form rendered as HTML <tr>s -- excluding the
<table></table>."
-        return
self._html_output(u'<tr><th>%(label)s</th><td>%(field)s</td></tr>',
u'<tr><td colspan="2">%s</td></tr>', '</td></tr>', True)
+        if errors_on_separate_row:
+            return
self._html_output(u'<tr><th>%(label)s</th><td>%(field)s</td></tr>',
+                                     u'<tr><td
colspan="2">%s</td></tr>', '</td></tr>', True)
+        else:
+            return
self._html_output(u'<tr><th>%(label)s</th><td>%(field)s</td></tr>',
+                                     u'<tr><th>%(label)s</th><td
class="error">%(error)s%(field)s</td></tr>', '</td></tr>', False)

     def as_ul(self):
         "Returns this form rendered as HTML <li>s -- excluding the
<ul></ul>."

Regards,
   Alex


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

Reply | Threaded
Open this post in threaded view
|

Re: newforms error design

Gary Wilson Jr.

[hidden email] wrote:
> But in fact it does not look user intuitive.

I concur with this statement.  I've got a form with about 10 fields and
when there are multiple errors for adjacent fields, it's very hard to
tell if the error is for the field above or below.

> The problem is visual. I do not like to present end users so ugly
> looking errors. As temp. solution I have patched as_table() output, so
> it takes optional argument:
>
> errors_on_separate_row

-0

My vote would be to change as_table() to make it more pleasing to the
eye rather than introduce a parameter.  The errors on a seperate row is
pretty unusable and we should not allow the creation of such forms in
Django :)


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

Reply | Threaded
Open this post in threaded view
|

Re: newforms error design

Adrian Holovaty-3
In reply to this post by alex.v.koval@gmail.com

On 12/22/06, [hidden email] <[hidden email]> wrote:
> The problem is visual. I do not like to present end users so ugly
> looking errors. As temp. solution I have patched as_table() output, so
> it takes optional argument:
>
> errors_on_separate_row
>
> So, now, when I call it from my application, I can specify to display
> errors on same row, what looks
> the following way (much more intuitive, IMO):

Hi Alex,

Thanks for this suggestion. I agree with you that putting the errors
in the same <td> as the field is a better idea. I've changed
Form.as_table() to do that in changeset [4239].

Adrian

--
Adrian Holovaty
holovaty.com | djangoproject.com

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