Improvements/fixes related to EditableDetailView. #16

Merged
merged 3 commits into from May 18, 2013

Conversation

Projects
None yet
2 participants
@mdeweerd
  • Allowing virtual attributes in EditableSaver.
  • Completed french translations.
  • Resolve translation limitation in EditableSaver.
  • Allowing default values for all EditableField attributes in EditableDetailView.

mdeweerd added some commits Mar 10, 2013

mdeweerd mdeweerd
- Allowing virutal attributes in EditableSaver.
- Completed french translations.
- Resolve translation limitation in EditableSaver.
- Allowing default values for all EditableField attributes in
EditableDetailView.
@vitalets

This comment has been minimized.

Show comment
Hide comment
@vitalets

vitalets Mar 14, 2013

Is the order in mergeArray correct? I suppose the latter array overwrites the former, and current options should have a priority under defaults.

Is the order in mergeArray correct? I suppose the latter array overwrites the former, and current options should have a priority under defaults.

This comment has been minimized.

Show comment
Hide comment
@mdeweerd

mdeweerd Mar 14, 2013

Owner

Your are right, I was concerned with not pulluting $this->_data with values from the field.
So it is better to make this something like:
$editable=CMap::mergeArray(array(),$this->_data); // Make a copy of the default options
if(isset($options['editable'])) $editable=CMap::mergeArray($editable,$options['editable']);
// Continue to use $editable instead of $options['editable'] ...

Owner

mdeweerd replied Mar 14, 2013

Your are right, I was concerned with not pulluting $this->_data with values from the field.
So it is better to make this something like:
$editable=CMap::mergeArray(array(),$this->_data); // Make a copy of the default options
if(isset($options['editable'])) $editable=CMap::mergeArray($editable,$options['editable']);
// Continue to use $editable instead of $options['editable'] ...

@vitalets

This comment has been minimized.

Show comment
Hide comment
@vitalets

vitalets Mar 14, 2013

@mdeweerd, thanks for great PR!
Some questions:

  1. Why you suggest to put defaults functionality into EditableField? from my understanding it's fully related to EditableDetailView and better be implemented there.
  2. What's the benefit of changing Yii::t('editable' to Yii::t('EditableSaver.editable' ?
  3. have a look on my comment in code about mergeArray arguments order.

@mdeweerd, thanks for great PR!
Some questions:

  1. Why you suggest to put defaults functionality into EditableField? from my understanding it's fully related to EditableDetailView and better be implemented there.
  2. What's the benefit of changing Yii::t('editable' to Yii::t('EditableSaver.editable' ?
  3. have a look on my comment in code about mergeArray arguments order.

This comment has been minimized.

Show comment
Hide comment
@mdeweerd

mdeweerd Mar 14, 2013

Owner
  1. The defaults are defined in EditableDetailView but the values that can be set are based on the properties of EditableField.
    The defaults are pushed to Editable field in 'renderItem' of the EditableDetailView.
  2. To make Yii find the 'messages//editable.php' from the location of the EditableSaver class. I had issues otherwise to apply the translation.
  3. Commented on your comment ;-).
Owner

mdeweerd replied Mar 14, 2013

  1. The defaults are defined in EditableDetailView but the values that can be set are based on the properties of EditableField.
    The defaults are pushed to Editable field in 'renderItem' of the EditableDetailView.
  2. To make Yii find the 'messages//editable.php' from the location of the EditableSaver class. I had issues otherwise to apply the translation.
  3. Commented on your comment ;-).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment