Enh #1238: CJuiDatePicker handling events (christiansalazar) #1239

Closed
wants to merge 20 commits into
from

Conversation

2 participants

Issue: #1238

primary:
widget CJuiDatePicker now is using an altField to update the hiddenField. Previously, the hiddenField was updated using the onSelect event but under this circunstances if you need this event (onSelect) then you must be carefull to update the hiddenField too in order to make it works, this commit will solve this making the hiddenField update via datepicker.altField.

secondary:
widget now addmit a function for every event using any of:
'onSelect'=>null,
'onSelect'=>"js:function(a,b){ ... }", (deprecated, but yet needed for datepicker)
'onSelect'=>"function(a,b){ ... }",
'onSelect'=>"myFunctionName",
'onSelect'=>new CJavaScriptExpression("function(a,b){ ... }"),

prior to this commit CJuiDatePicker only addmits:
'onSelect'=>"js:function(a,b){ ... }", (using "js:", deprecated, or it wont be recognized by datepicker as a function)

internally the widget will process the value passed for every event converting it to an instance of CJavaScriptExpression.

Please group this line with other "Enh" lines... ordered by numbers

empty line?

sorry for this.
now i moved it into version 1.1.12 section as a last -Enh- entry.

why not use this together on one if

if(isset($this->options['onSelect']) && !($this->options['onSelect'] instanceof CJavaScriptExpression))

what would be the case of user setting his own altField?

By setting the altField the hidden input would not work anymore, so I'm not sure if we need to check this.

note about: (trim($this->options['onSelect']) != '') &&

is needed because when is initialized as this:
onSelect=>"",

this will cause:
CJavaScript::encode(new CJavaScriptExpression(""))
in consecuence, encode will output:
onSelect:,
..it will crash javascript because is a malformed option..

so, filtering for an empty string will avoid this situation producing:
onSelect:"",
when an empty string is provided.

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

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

going more deeply yet: all derived classes from CJuiWidget needs this fix.

simple:
a base method: CJuiWidget::normalizeEvents(eventsArray) will resolve it.

CJuiDatePicker:
self::normalizeEvents(array('onSelect','onClose','onChangeMonthYear','beforeShowDay','beforeShow'));
CJuiTabs:
self::normalizeEvents(array('create','load','show','add','remove','enable','disable'));

and so on, for each derived class.

Member

mdomba commented Aug 20, 2012

Not sure if this is needed... we are talking about just 3 lines of code... IMO a separate method for those is not needed... I don't see a problem in having those 3 lines in all widgets that needs it.

ok.
this change will consume a liitle bit more time in testing, because ensure all events works fine. at least in datetime case all events works fine right now, expected the same for remaining widgets in cjui package.

Member

mdomba commented Aug 21, 2012

For now we can merge this one, later you can create a new PR for other widgets...

You just need to update the changelog - as 1.1.12 has been released you need to move your description to the 1.1.13 section.

please only pay attention to commit: 4a3554c

why did you put this on the same line, Yii core code has similar statements on 2 lines as it was.

sorry, it was a mistake.

mdomba was assigned Sep 8, 2012

mdomba closed this Nov 20, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment