Code

Opened 3 years ago

Closed 3 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@…> 3 years ago.
splitdatetimewidget.2.patch (1.1 KB) - added by slink <gabor@…> 3 years ago.

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by slink <gabor@…>

comment:1 follow-up: Changed 3 years ago by akaariai

  • 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 3 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 3 years ago by slink <gabor@…>

comment:3 Changed 3 years ago by akaariai

  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design 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 3 years ago by jaddison

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 3 years ago by julien

  • Easy pickings unset
  • Resolution set to duplicate
  • Status changed from new to closed
  • 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.

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.