From bdb1a957583662fb0c463f61fee70cfefd26c988 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 13 Feb 2015 17:35:39 +1300 Subject: [PATCH 1/2] API Cleanup and refactor of select fields API Standardise Relation interface --- .../thirdparty/chosen/chosen/chosen.jquery.js | 10 +- docs/en/04_Changelogs/4.0.0.md | 15 + forms/CheckboxSetField.php | 310 ++---------------- forms/CountryDropdownField.php | 76 +++-- forms/DropdownField.php | 273 ++------------- forms/FormField.php | 1 - forms/GroupedDropdownField.php | 114 +++---- forms/ListboxField.php | 286 ++++------------ forms/LookupField.php | 37 +-- forms/MemberDatetimeOptionsetField.php | 144 ++++---- forms/MultiSelectField.php | 221 +++++++++++++ forms/OptionsetField.php | 65 ++-- forms/SelectField.php | 212 ++++++++++++ forms/SingleSelectField.php | 121 +++++++ model/Relation.php | 43 +++ model/RelationList.php | 4 +- model/UnsavedRelationList.php | 173 +--------- security/Group.php | 1 - security/Member.php | 1 - .../Includes/GroupedDropdownFieldOption.ss | 12 + templates/forms/DropdownField.ss | 5 +- templates/forms/GroupedDropdownField.ss | 5 + templates/forms/ListboxField.ss | 5 + .../forms/MemberDatetimeOptionsetField.ss | 14 + tests/forms/CheckboxSetFieldTest.php | 70 +++- tests/forms/DropdownFieldTest.php | 31 ++ tests/forms/GroupedDropdownFieldTest.php | 9 +- tests/forms/ListboxFieldTest.php | 60 +--- .../MemberDatetimeOptionsetFieldTest.php | 15 +- 29 files changed, 1087 insertions(+), 1246 deletions(-) create mode 100644 forms/MultiSelectField.php create mode 100644 forms/SelectField.php create mode 100644 forms/SingleSelectField.php create mode 100644 model/Relation.php create mode 100644 templates/Includes/GroupedDropdownFieldOption.ss create mode 100644 templates/forms/GroupedDropdownField.ss create mode 100644 templates/forms/ListboxField.ss create mode 100644 templates/forms/MemberDatetimeOptionsetField.ss diff --git a/admin/thirdparty/chosen/chosen/chosen.jquery.js b/admin/thirdparty/chosen/chosen/chosen.jquery.js index e5f598383..5ce0f156f 100644 --- a/admin/thirdparty/chosen/chosen/chosen.jquery.js +++ b/admin/thirdparty/chosen/chosen/chosen.jquery.js @@ -49,7 +49,8 @@ SelectParser.prototype.add_option = function(option, group_position, group_disabled) { if (option.nodeName.toUpperCase() === "OPTION") { - if (option.text !== "") { + // workaround for https://github.com/harvesthq/chosen/issues/2125 + if (!option.text.match(/^\s*$/g)) { if (group_position != null) { this.parsed[group_position].children += 1; } @@ -135,7 +136,12 @@ Copyright (c) 2011 by Harvest this.results_showing = false; this.result_highlighted = null; this.result_single_selected = null; - this.allow_single_deselect = (this.options.allow_single_deselect != null) && (this.form_field.options[0] != null) && this.form_field.options[0].text === "" ? this.options.allow_single_deselect : false; + this.allow_single_deselect = (this.options.allow_single_deselect != null) + && (this.form_field.options[0] != null) + // workaround for https://github.com/harvesthq/chosen/issues/2125 + && this.form_field.options[0].text.match(/^\s*$/g) + ? this.options.allow_single_deselect + : false; this.disable_search_threshold = this.options.disable_search_threshold || 0; this.disable_search = this.options.disable_search || false; this.search_contains = this.options.search_contains || false; diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 5ed6215a4..81df11fce 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -498,3 +498,18 @@ configuration customisation is done via overriding these templates. If upgrading from an existing installation, make sure to invoke ?flush=all at least once. See [/developer_guides/files/file_security] for more information. + +### `ListboxField` is now multiple-only + +Previously, this field would operate as either a single select (default) or multi-select by setting +`setMultiple` to either true or false. + +Now this field should only be used for multi-selection. Single-selection should be done using +a regular `DropdownField`. + +### `GroupedDropdownField::setDisabled` now only accepts a list of values. + +Where previously you could specify a list of grouped values in the same way as `setSource`, this +method now only accepts either a non-associative array of values (not titles) or an `SS_List` +of items to disable. + diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php index e72b1f516..d12f63519 100644 --- a/forms/CheckboxSetField.php +++ b/forms/CheckboxSetField.php @@ -36,12 +36,7 @@ * @package forms * @subpackage fields-basic */ -class CheckboxSetField extends OptionsetField { - - /** - * @var array - */ - protected $defaultItems = array(); +class CheckboxSetField extends MultiSelectField { /** * @todo Explain different source data that can be used with this field, @@ -60,306 +55,43 @@ class CheckboxSetField extends OptionsetField { } /** + * Gets the list of options to render in this formfield + * * @return ArrayList */ public function getOptions() { + $selectedValues = $this->getValueArray(); + $defaultItems = $this->getDefaultItems(); + + // Generate list of options to display $odd = 0; - - $source = $this->source; - $values = $this->value; - $items = array(); - - // Get values from the join, if available - if(is_object($this->form)) { - $record = $this->form->getRecord(); - if(!$values && $record && $record->hasMethod($this->name)) { - $funcName = $this->name; - $join = $record->$funcName(); - if($join) { - foreach($join as $joinItem) { - $values[] = $joinItem->ID; - } - } - } - } - - // Source is not an array - if(!is_array($source) && !is_a($source, 'SQLMap')) { - if(is_array($values)) { - $items = $values; - } else { - // Source and values are DataObject sets. - if($values && is_a($values, 'SS_List')) { - foreach($values as $object) { - if(is_a($object, 'DataObject')) { - $items[] = $object->ID; - } - } - } elseif($values && is_string($values)) { - if(!empty($values)) { - $items = explode(',', $values); - $items = str_replace('{comma}', ',', $items); - } else { - $items = array(); - } - } - } - } else { - // Sometimes we pass a singluar default value thats ! an array && !SS_List - if($values instanceof SS_List || is_array($values)) { - $items = $values; - } else { - if($values === null) { - $items = array(); - } - else { - if(!empty($values)) { - $items = explode(',', $values); - $items = str_replace('{comma}', ',', $items); - } else { - $items = array(); - } - } - } - } - - if(is_array($source)) { - unset($source['']); - } - - $options = array(); - - if ($source == null) { - $source = array(); - } - - foreach($source as $value => $item) { - if($item instanceof DataObject) { - $value = $item->ID; - $title = $item->Title; - } else { - $title = $item; - } - - $itemID = $this->ID() . '_' . preg_replace('/[^a-zA-Z0-9]/', '', $value); + $formID = $this->ID(); + $options = new ArrayList(); + foreach($this->getSource() as $itemValue => $title) { + $itemID = Convert::raw2htmlid("{$formID}_{$itemValue}"); $odd = ($odd + 1) % 2; $extraClass = $odd ? 'odd' : 'even'; - $extraClass .= ' val' . preg_replace('/[^a-zA-Z0-9\-\_]/', '_', $value); + $extraClass .= ' val' . preg_replace('/[^a-zA-Z0-9\-\_]/', '_', $itemValue); + + $itemChecked = in_array($itemValue, $selectedValues) || in_array($itemValue, $defaultItems); + $itemDisabled = $this->isDisabled() || in_array($itemValue, $defaultItems); - $options[] = new ArrayData(array( + $options->push(new ArrayData(array( 'ID' => $itemID, 'Class' => $extraClass, - 'Name' => "{$this->name}[{$value}]", - 'Value' => $value, + 'Name' => "{$this->name}[{$itemValue}]", + 'Value' => $itemValue, 'Title' => $title, - 'isChecked' => in_array($value, $items) || in_array($value, $this->defaultItems), - 'isDisabled' => $this->disabled || in_array($value, $this->disabledItems) - )); + 'isChecked' => $itemChecked, + 'isDisabled' => $itemDisabled, + ))); } - - $options = new ArrayList($options); - $this->extend('updateGetOptions', $options); - return $options; } - /** - * Default selections, regardless of the {@link setValue()} settings. - * Note: Items marked as disabled through {@link setDisabledItems()} can still be - * selected by default through this method. - * - * @param Array $items Collection of array keys, as defined in the $source array - */ - public function setDefaultItems($items) { - $this->defaultItems = $items; - return $this; - } - - /** - * @return Array - */ - public function getDefaultItems() { - return $this->defaultItems; - } - - /** - * Load a value into this CheckboxSetField - */ - public function setValue($value, $obj = null) { - // If we're not passed a value directly, we can look for it in a relation method on the object passed as a - // second arg - if(!$value && $obj && $obj instanceof DataObject && $obj->hasMethod($this->name)) { - $funcName = $this->name; - $value = $obj->$funcName()->getIDList(); - } - - parent::setValue($value, $obj); - - return $this; - } - - /** - * Save the current value of this CheckboxSetField into a DataObject. - * If the field it is saving to is a has_many or many_many relationship, - * it is saved by setByIDList(), otherwise it creates a comma separated - * list for a standard DB text/varchar field. - * - * @param DataObject $record The record to save into - */ - public function saveInto(DataObjectInterface $record) { - $fieldname = $this->name; - $relation = ($fieldname && $record && $record->hasMethod($fieldname)) ? $record->$fieldname() : null; - if($fieldname && $record && $relation && - ($relation instanceof RelationList || $relation instanceof UnsavedRelationList)) { - $idList = array(); - if($this->value) foreach($this->value as $id => $bool) { - if($bool) { - $idList[] = $id; - } - } - $relation->setByIDList($idList); - } elseif($fieldname && $record) { - if($this->value) { - $this->value = str_replace(',', '{comma}', $this->value); - $record->$fieldname = implode(',', (array) $this->value); - } else { - $record->$fieldname = ''; - } - } - } - - /** - * Return the CheckboxSetField value as a string - * selected item keys. - * - * @return string - */ - public function dataValue() { - if($this->value && is_array($this->value)) { - $filtered = array(); - foreach($this->value as $item) { - if($item) { - $filtered[] = str_replace(",", "{comma}", $item); - } - } - - return implode(',', $filtered); - } - - return ''; - } - - public function performDisabledTransformation() { - $clone = clone $this; - $clone->setDisabled(true); - - return $clone; - } - - /** - * Transforms the source data for this CheckboxSetField - * into a comma separated list of values. - * - * @return ReadonlyField - */ - public function performReadonlyTransformation() { - $values = ''; - $data = array(); - - $items = $this->value; - if($this->source) { - foreach($this->source as $source) { - if(is_object($source)) { - $sourceTitles[$source->ID] = $source->Title; - } - } - } - - if($items) { - // Items is a DO Set - if($items instanceof SS_List) { - foreach($items as $item) { - $data[] = $item->Title; - } - if($data) $values = implode(', ', $data); - - // Items is an array or single piece of string (including comma seperated string) - } else { - if(!is_array($items)) { - $items = preg_split('/ *, */', trim($items)); - } - - foreach($items as $item) { - if(is_array($item)) { - $data[] = $item['Title']; - } elseif(is_array($this->source) && !empty($this->source[$item])) { - $data[] = $this->source[$item]; - } elseif(is_a($this->source, 'SS_List')) { - $data[] = $sourceTitles[$item]; - } else { - $data[] = $item; - } - } - - $values = implode(', ', $data); - } - } - - $field = $this->castedCopy('ReadonlyField'); - $field->setValue($values); - - return $field; - } - public function Type() { return 'optionset checkboxset'; } - public function ExtraOptions() { - return FormField::ExtraOptions(); - } - - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) { - $values = $this->value; - if (!$values) { - return true; - } - $sourceArray = $this->getSourceAsArray(); - if (is_array($values)) { - if (!array_intersect_key($sourceArray, $values)) { - $validator->validationError( - $this->name, - _t( - 'CheckboxSetField.SOURCE_VALIDATION', - "Please select a value within the list provided. '{value}' is not a valid option", - array('value' => implode(' and ', array_diff($sourceArray, $values))) - ), - "validation" - ); - return false; - } - } else { - if (!in_array($this->value, $sourceArray)) { - $validator->validationError( - $this->name, - _t( - 'CheckboxSetField.SOURCE_VALIDATION', - "Please select a value within the list provided. '{value}' is not a valid option", - array('value' => $this->value) - ), - "validation" - ); - return false; - } - } - return true; - } - } diff --git a/forms/CountryDropdownField.php b/forms/CountryDropdownField.php index 61a5f0e83..e68dd063d 100644 --- a/forms/CountryDropdownField.php +++ b/forms/CountryDropdownField.php @@ -11,13 +11,18 @@ class CountryDropdownField extends DropdownField { /** * Should we default the dropdown to the region determined from the user's locale? + * + * @config * @var bool */ private static $default_to_locale = true; /** - * The region code to default to if default_to_locale is set to false, or we can't determine a region from a locale - * @var string + * The region code to default to if default_to_locale is set to false, or we can't + * determine a region from a locale. + * + * @config + * @var string */ private static $default_country = 'NZ'; @@ -28,48 +33,57 @@ class CountryDropdownField extends DropdownField { * @return string */ protected function locale() { - if (($member = Member::currentUser()) && $member->Locale) return $member->Locale; + if (($member = Member::currentUser()) && $member->Locale) { + return $member->Locale; + } return i18n::get_locale(); } - public function __construct($name, $title = null, $source = null, $value = "", $form=null) { - if(!is_array($source)) { - // Get a list of countries from Zend - $source = Zend_Locale::getTranslationList('territory', $this->locale(), 2); - - // We want them ordered by display name, not country code - - // PHP 5.3 has an extension that sorts UTF-8 strings correctly - if (class_exists('Collator') && ($collator = Collator::create($this->locale()))) { - $collator->asort($source); - } - // Otherwise just put up with them being weirdly ordered for now - else { - asort($source); - } - - // We don't want "unknown country" as an option - unset($source['ZZ']); + public function setSource($source) { + if($source) { + return parent::setSource($source); } - parent::__construct($name, ($title===null) ? $name : $title, $source, $value, $form); + // map empty source to country list + // Get a list of countries from Zend + $source = Zend_Locale::getTranslationList('territory', $this->locale(), 2); + + // We want them ordered by display name, not country code + + // PHP 5.3 has an extension that sorts UTF-8 strings correctly + if (class_exists('Collator') && ($collator = Collator::create($this->locale()))) { + $collator->asort($source); + } else { + // Otherwise just put up with them being weirdly ordered for now + asort($source); + } + + // We don't want "unknown country" as an option + unset($source['ZZ']); + + return parent::setSource($source); } public function Field($properties = array()) { $source = $this->getSource(); - if (!$this->value || !isset($source[$this->value])) { - if ($this->config()->default_to_locale && $this->locale()) { - $locale = new Zend_Locale(); - $locale->setLocale($this->locale()); - $this->value = $locale->getRegion(); - } + // Default value to best availabel locale + $value = $this->Value(); + if ($this->config()->default_to_locale + && (!$value || !isset($source[$value])) + && $this->locale() + ) { + $locale = new Zend_Locale(); + $locale->setLocale($this->locale()); + $value = $locale->getRegion(); + $this->setValue($value); } - if (!$this->value || !isset($source[$this->value])) { - $this->value = $this->config()->default_country; + // Default to default country otherwise + if (!$value || !isset($source[$value])) { + $this->setValue($this->config()->default_country); } - return parent::Field(); + return parent::Field($properties); } } diff --git a/forms/DropdownField.php b/forms/DropdownField.php index 60902a019..c4f8a3016 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -81,51 +81,31 @@ * @package forms * @subpackage fields-basic */ -class DropdownField extends FormField { +class DropdownField extends SingleSelectField { /** - * @var array|ArrayAccess $source Associative or numeric array of all dropdown items, - * with array key as the submitted field value, and the array value as a - * natural language description shown in the interface element. + * Build a field option for template rendering + * + * @param mixed $value Value of the option + * @param string $title Title of the option + * @return ArrayData Field option */ - protected $source; + protected function getFieldOption($value, $title) { + // Check selection + $selected = $this->isSelectedValue($value, $this->Value()); - /** - * @var boolean $isSelected Determines if the field was selected - * at the time it was rendered, so if {@link $value} matches on of the array - * values specified in {@link $source} - */ - protected $isSelected; + // Check disabled + $disabled = false; + if(in_array($value, $this->getDisabledItems()) && $title != $this->getEmptyString()){ + $disabled = 'disabled'; + } - /** - * @var boolean $hasEmptyDefault Show the first