(new) CJuiDatePicker handling native datepicker events.

[christiansalazar]

The original source provided in CJuiDatePicker uses the onSelect (datepicker native event) to pass selected date into hiddenField in order to be consistent in model/attribute scenario, but, if you try use this onSelect event for your own pruposes then you must be carefull to support the hiddenField update or your model doesnt receive the selected date.

The primary target in this commit is to separate the hiddenField update from the event onSelect, and as secondary target the commit will provide support for all event handlers found currently in datepicker jQuery component.

1. onSelect
2. onClose
3. onChangeMonthYear
4. beforeShowDay
5. beforeShow

Yes, the current CJuiDatePicker support this events via passing it from options, but events will works only prepending "js:" in the function passed via string:
"options"=>array(
"onSelect"=>"js:myHandler(a,b){ ... }"
),
(it doesnt works if you pass: "onSelect"=>"myHandler(a,b){ ... }", without the 'js:')
the "js" is deprecated.

This commit will solve this two targets (hiddenField update and event processign):

1. hiddenField:
   will be updated using the altField provided by datepicker, leaving onSelect event for user defined pruposes.

2 event processing:
all events mentioned before will be processed in order to be a CJavaScriptExpression instance, instead of a simple string,
so, this cases will work fine now:

'onSelect'=>null,
'onSelect'=>"js:function(a,b){ ... }", (deprecated, but yet needed for datepicker)
'onSelect'=>"function(a,b){ ... }",
'onSelect'=>new CJavaScriptExpression("function(a,b){ ... }"),

important:
'onSelect'=>"",
will produce a javascript crash because a CJavaScript::encode(new CJavaScriptExpression("")) will produce an invalid option for datepicker.

[mdomba]

The "js:" method is deprecated and should not be used anymore... so this part should be removed...

Please explain why would we need to add the onSelect property when you can already send a custom handler with options['onSelect'] ?

[christiansalazar]

if you override the default "options['onSelect']" functionality then you must remembero to notify the new selectedDate value to the hiddenField (model / attribute case), if you forgot or misplace something then the model/attribute does not receive the selected value and modeling fails.
using this way (widget.onSelect) i take care of this hidden field to support the model/attribute mechanism.

Finally, the standard yii programmer can use this widget in a more clear and simple way instead of writing code to support the selectedDate to be passed to model.attribute.

[christiansalazar]

in respect to "js:", it is handled on widget in case the user doesn't know about deprecation. I could throw an exception if you prefer to ensure the user must not use "js:" anymore.
If user doesn't use "js:" the widget works fine because it first looks for "function(" keyword to detect if this funcion is invoked by functionName or by inline declaration, always removing the "js:" using Ltrim(), prior to usage. Any suggest ?

[mdomba]

"js:" is already handled by encode method so you don't need to check that at all

but...

I think I found here a better solution. The main point in this issue is to fill the hidden field so that developers do not need to think about it, right ?

How about using the altField for this ?

The solution is as simple as replacing the lines 105-106 - https://github.com/yiisoft/yii/blob/master/framework/zii/widgets/jui/CJuiDatePicker.php#L105

if (!isset($this->options['onSelect']))
    $this->options['onSelect']=new CJavaScriptExpression("function( selectedDate ) { jQuery('#{$id}').val(selectedDate);}");

with

$this->options['altField']='#'.$id;

[christiansalazar]

Yes you're in right. the provided altField (core datepicker ui options) is there to update an extra field and not only for common ALT usage. next commit will use this field to handle the hidden field, leaving the onSelect option free for directly usage without taking care of hiddenField value.

[christiansalazar]

last commit ready implementing altField and a filter for onSelect, please read more about this:

it works fine as:

$this->widget('zii.widgets.jui.CJuiDatePicker', array(
'options'=>array(
'onSelect'=>"function(dateSelected,inst){ ... }",
'firstDay'=>1,
),
));

and it works fine too using CJavaScriptExpression:

$this->widget('zii.widgets.jui.CJuiDatePicker', array(
'options'=>array(
'onSelect'=>new CJavaScriptExpression("function(dateSelected,inst){ ... }"),
'firstDay'=>1,
),
));

if the final user use or not the CJavaScriptExpression to set a handler for onSelect option then CJuiDatePicker take care of this verifying if the passed onSelect value is an instanceOf CJavaScriptExpression, if not it make it a new CJavaScriptExpression using the provided value, so the resulting script is:
'onSelect':function(dateSelected,inst){ ... },
in both cases.

[mdomba]

The only needed change is as I wrote it above... only one line!

As I already explained before.. you don't need to check the options... all that is done already by encode method - https://github.com/christiansalazar/yii/blob/fb702a28f47f812961b6ce95f4d43a4316788730/framework/zii/widgets/jui/CJuiDatePicker.php#L136

and there is no need to add inline comments explaining the altField usage

[christiansalazar]

comment lines removed, but, in the case of onSelect option, the provided code is needed, because:

if we dont process the onSelect option, this piece of code doesnt works.
'onSelect'=>'function(a,b){ ... }'

because CJavascript::encode() threat it as a string, producing:
{'onSelect':"function(a,b){ ... }"}
and datepicker do not recognize it (if it is a string, it must be prepended with a js)

the way1 to make it works is: (deprecated, prepending js)
'onSelect'=>'js:function(a,b){ ... }'

the way2 to make it works (right way):
'onSelect'=>new CJavascriptExpression('function(a,b){ ... }'),

but, the standard yii programmer doesnt know the way2. so, in order to help, the widget would detect if he or she is using a string or an instance of CJavascriptExpression, if is not a CJavascriptExpression then it convert the provided string into a CJavascriptExpression, so:

the way3: (a pure string, now it will works fine)
'onSelect'=>'function(a,b){ ... }'

As a resume, processing the onSelect option now will allow this usage:
a) 'onSelect'=>'function(a,b){ ... }'
b) 'onSelect'=>'js:function(a,b){ ... }'
c) 'onSelect'=>new CJavascriptExpression('function(a,b){ ... }'),
(if dont, the unique code wich will works would be: b and c, and the most simplest of them (a) will not works ..! )

[mdomba]

yes, you are right for the onSelect processing... I made two comments on the latest commit..

[christiansalazar]

latest commit has this change on it, plus: an extra validation for: "onSelect"=>"". needed because if this event handler is an empty string then the widget disappears.

example (whitout filtering for empty string):
'onSelect'=>'',
produces:
{'onSelect':,'firstDay':1, (code extract produced by CJavascript::encode, note the: ":,"), it will crash javascript.

is not the same case when passing null:
'onSelect'=>null,
produces:
{'onSelect':null,'firstDay':1,
it will works fine.

so the extra validation for empty string is to avoid the javascript crash produced when CJavascript::encode outputs: nothing when it receive an instance of: new CJavaScriptExpression(""). (not same case with null, it outputs null)

[mdomba]

that is a developer error... why would we fix that?

it would be the same for any other value or not ?

[christiansalazar]

at this point, the input test for onSelect is OK for this cases:

'onSelect'=>null,
'onSelect'=>"",
'onSelect'=>"js:function(a,b){ ... }", (deprecated, but yet needed for datepicker)
'onSelect'=>"function(a,b){ ... }",
'onSelect'=>new CJavaScriptExpression("function(a,b){ ... }"),

[christiansalazar]

in respect to:

"that is a developer error... why would we fix that?
it would be the same for any other value or not ?" (let me review the other event handlers)

i think: is not an error to provide an empty string for an onSelect=>"", i think it will confuse the developper: he or she will pass hours detecting why javascript is crash, and when he or she will be wondered when discover the cause: a simple empty string.

[christiansalazar]

going more deeply, the next events will be processed in the same way as for onSelect case:

1. onSelect (ready)
2. onClose
3. onChangeMonthYear
4. beforeShowDay
5. beforeShow

[mdomba]

My point is that it would be the same for any other Yii properties that requires javascript code...

and btw why would anyone set this to an empty string?

The documentation for this option is very clear - http://jqueryui.com/demos/datepicker/#event-onSelect

Supply a callback function to handle the onSelect event as an init option.

[christiansalazar]

let me find an existing case to verify it,
i think it must be exist a method who receive an event handler name and apply the procedure for each of them, so if you decide not supporting the empty string it will apply easily for each of them.
$this->filterEventHandlers({"onSelect","onClose",..,...,})

[mdomba]

Again... why would we do it ?
why would anyone set this to an empty string?

what is your use-case for this?

[christiansalazar]

maurizio, looking in CGridView.selectionChanged when an empty string is provided it will crash too in same way. So, as you said, we dont fix the empty string in CJui too.

[mdomba]

the job of the framework is to help developers to do things faster... it's not to fix developers errors ;)

[christiansalazar]

yes you are on right.
next commit will remove the empty string validation.

next question: supporting the same filtering for:

1. onSelect (ready)
2. onClose
3. onChangeMonthYear
4. beforeShowDay
5. beforeShow

i think to provide a method for this.

[mdomba]

for that (more events) you can use this code - https://github.com/yiisoft/yii/blob/master/framework/web/helpers/CHtml.php#L1048

[christiansalazar]

test for all datepicker events passed.

[christiansalazar]

nice.
i see the issue was reduced only to the altField improvment.
do you think the changes for events handled by CJavaScriptExpression in the whole jui library could be applied later ?

[mdomba]

yes, there is no need for any special event handling as it's already handled when the JS code is used with 'js:' or 'new CJavascriptExpression'

There is no need to handle the variation with pure JS code ((like you tryed) ) because in all other parts of the framework where JS code can be assigned the 'js:' (deprecated) or CJavascriptExpression (prefered) should be used.

[shorif2000]

hello. i am trying to get selectWeek to work. can anyone help? http://stackoverflow.com/questions/26713040/cjuidatepicker-select-week

[christiansalazar]

hi shorif2000, im sure this is not the right place to put your issue. :-) also im sure this issue is closed.