Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #370 (closed defect: fixed)

Opened 13 years ago

Last modified 12 years ago

[PATCH] CalendarDateTimePicker fills in current date/time even when given default=None

Reported by: nerkles Owned by: anonymous
Priority: normal Milestone: 0.9
Component: TG Widgets Version:
Severity: major Keywords: widgets date time
Cc:

Description

You just can't leave the field blank. I've confirmed that the database records have NULL in the column, and the model.py specifies default=None. The widget is fed the correct value, but it still fills in the current date/time anyway.

Looks like the problem starts on line 528 in widgets/base.py, although there may be other places it causes trouble. I just commented out the two lines and it seems to solve the problem without creating any new ones. So I suggest simply deleting them.

def _get_default(self):

# if self._default is None:

# return datetime.now()

return self._default

There are a lot of circumstances where the current date/time as default would make sense, but I can think of an important one where it is not OK: the expiration time for content in a CMS. If the current date/time is filled in here, your content expires the moment you create it. And a lot content is permanent, and has no need to specify an expiration, hence the need to be able to leave it blank.

But in any case, all widgets should use the default value you feed them, with the possible exception of bogus data.

Attachments

CalendarDatePicker-accept-empty.patch Download (3.4 KB) - added by spleeman@… 13 years ago.
Patch for CalendarDatePicker? widget to allow empty values
CalendarDatePicker-template-update.patch Download (750 bytes) - added by spleeman@… 13 years ago.
Reverts a few changes to the template of the CalendarDatePicker? widget.

Change History

comment:1 Changed 13 years ago by spleeman@…

Patch attached that allows the CalendarDate?[Time]Picker to have be empty. Backwards compatible.

Adds a new keyword on init - allow_empty=[True|False]

e.g.

CalendarDatePicker(name="start_date", labeltext="Start Date", format="%d/%m/%Y", allow_empty=True)

Changed 13 years ago by spleeman@…

Patch for CalendarDatePicker? widget to allow empty values

comment:2 Changed 13 years ago by anonymous

  • Summary changed from CalendarDateTimePicker fills in current date/time even when given default=None to [PATCH] CalendarDateTimePicker fills in current date/time even when given default=None

comment:3 Changed 13 years ago by elvelind@…

  • Status changed from new to closed
  • Resolution set to fixed

fixed in 584

comment:4 Changed 13 years ago by Jorge Godoy <jgodoy@…>

I solved this problem by passing an empty string () as the value. But I agree that if it was None nothing should be put in there and the empty string is ugly (specially because the model/SQLObject returns None for NULL values).

I suggest reverting the changes you made to lines 537 and 538:

(from) 537 <input type="text" id="${widget.name}" name="${widget.name}" value="${widget_value}" 538 class="date_field ${' '.join(widget.css_classes)}" py:attrs="widget.attrs"/>

(to)

537 <input type="text" id="${widget.name}" name="${widget.name}" value="${widget_value}" class="date_field"/>

because it would break the implementation made to allow different CSS classes on the widget.

comment:5 Changed 13 years ago by spleeman@…

"because it would break the implementation made to allow different CSS classes on the widget."

I don't see how it would break any implimentation for the different CSS classes on the widget. The code I've used there was on the original widget and also is on other widgets in base.py.

Can you show me an example of this implimentation in case other widgets are also lagging behind and I'm using old code as a reference?

comment:6 Changed 13 years ago by Jorge Godoy <jgodoy@…>

> I don't see how it would break any implimentation for the different
> CSS classes on the widget. The code I've used there was on the
> original widget and also is on other widgets in base.py.

Take a look at #413 and at the link cited there:  http://groups.google.com/group/turbogears/browse_thread/thread/5ebe0dc0bda1f5df/e3c6783a526f1cb1#e3c6783a526f1cb1

The code I cited on the ticket was removed only from CalendarDatePicker? and was left on all other widgets.

> Can you show me an example of this implimentation in case other
> widgets are also lagging behind and I'm using old code as a reference?

All other have it. You can take a look at line 45 for Widget, line 56 for its init method, line 76/77 still in init, line 295 for Label, line 305 for TextField?, line 313 for PasswordField?, line 321 for ButtonField?, line 329 for ImageField?, line 337 for FileField?, line 346 for TextArea?, line 362 for FieldSet?, line 435 for SelectField?, line 452 for Checkable, line 483 for CheckableList? and at line 595 of base.py for AutoCompleteField. All these implement what has been removed from CalendarDatePicker?.

comment:7 Changed 13 years ago by michele

Anyway I still can't see the css_classes utility, you get the same result using attrs. And if it doesn't give you the expected result than it should be fixed.

There should be one-- and preferably only one --obvious way to do it.

comment:8 Changed 13 years ago by Jorge Godoy <jgodoy@…>

The difference with using attrs is having to copy the original class or not. I think this specialization does no harm and makes it easy for the developer. He can sum his CSS and apply things for the original class and for his specialized class without having to add extra classes (the original class) to his code. It helps decoupling whatever class is used by default on each and every widget from what is added in code. attrs is still valid to replace everything and leave just what the developer of the app wanted.

I see utility for both here, with different purposes.

comment:9 Changed 13 years ago by spleeman@…

  • Status changed from closed to reopened
  • Resolution fixed deleted

I've submitted a patch and reopened the ticket to change the template to be inline with this (I think).

Sorry I didn't understand your comment before. :)

Changed 13 years ago by spleeman@…

Reverts a few changes to the template of the CalendarDatePicker? widget.

comment:10 Changed 13 years ago by elvelind@…

  • Status changed from reopened to closed
  • Resolution set to fixed

Commited again :)

Note: See TracTickets for help on using tickets.