Opened 6 years ago

Closed 5 years ago

#14718 closed New feature (duplicate)

Different attributes for DateInput and TimeInput in SplitDateTimeWidget

Reported by: slink <gabor@…> Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords: widget date time
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Current implementation of SplitDateTimeWidget accepts an attrs dictionary and passes it to both DateInput and TimeInput widgets. It would be nice if widgets may have different attributes while attrs still applies to both.

For instance:

widget = SplitDateTimeWidget(attrs={'class': 'input-field'},
                             date_attrs={'dojoType': 'dijit.form.DateTextBox'},
                             time_attrs={'dojoType': 'dijit.form.DateTextBox'})

Attachments (2)

splitdatetimewidget.patch (1.1 KB) - added by slink <gabor@…> 6 years ago.
splitdatetimewidget.2.patch (1.1 KB) - added by slink <gabor@…> 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by slink <gabor@…>

Attachment: splitdatetimewidget.patch added

comment:1 Changed 6 years ago by Anssi Kääriäinen

Needs documentation: set
Needs tests: set
Patch needs improvement: set

I think time_attrs and date_attrs need to be last in the init argument list for backwards compatibility, it is possible that the init is called in this way

SplitDateTimeWidget({'foo': 'bar'}, 'date_format_string', 'time_format_string')

This patch will of course break that usage.

comment:2 in reply to:  1 Changed 6 years ago by slink <gabor@…>

Replying to akaariai:

I think time_attrs and date_attrs need to be last in the init argument list for backwards compatibility, it is possible that the init is called in this way

SplitDateTimeWidget({'foo': 'bar'}, 'date_format_string', 'time_format_string')

This patch will of course break that usage.

You're right. They should be in the end of the argment list.

Changed 6 years ago by slink <gabor@…>

Attachment: splitdatetimewidget.2.patch added

comment:3 Changed 6 years ago by Anssi Kääriäinen

Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

Looks good to me. Now this ticket needs tests and documentation. I am marking this as DDN, although I can't see a reason not to accept this. But I don't know if I am allowed to accept this ticket...

There is also a question about if things defined in attrs should override date_attrs (as current patch implements it) or if it should be the other way. It feels more logical that the more specific dict is preferred, that is attributes in date_attrs should override attributes in attrs. However this doesn't seem too important, and is probably fine as is. I can't think of a use case where this would be a problem.

comment:4 Changed 5 years ago by James Addison

Severity: Normal
Type: New feature

comment:5 Changed 5 years ago by Julien Phalip

Easy pickings: unset
Resolution: duplicate
Status: newclosed
UI/UX: unset

This is in fact a duplicate of #5851. Note that the latest suggested approach is to add this functionality in a more general way inside MultiWidget.

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