From 4367e9641356b13764aceb289c46b6023c6c6bb8 Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Sat, 20 Jun 2020 13:14:42 -0400 Subject: [PATCH 1/4] Attributes for a form element should always be an array --- CRM/Admin/Form/Job.php | 2 +- CRM/Admin/Form/MessageTemplates.php | 4 +- CRM/Admin/Form/PdfFormats.php | 2 +- CRM/Contact/Form/Task/EmailTrait.php | 2 +- .../Form/ContributionPage/Premium.php | 2 +- CRM/Contribute/Form/ManagePremiums.php | 6 +- CRM/Core/BAO/CustomField.php | 143 +++++++++--------- CRM/Core/Form.php | 11 +- CRM/Import/DataSource/SQL.php | 2 +- CRM/Price/Form/Field.php | 2 +- CRM/SMS/Form/Provider.php | 2 +- 11 files changed, 91 insertions(+), 87 deletions(-) diff --git a/CRM/Admin/Form/Job.php b/CRM/Admin/Form/Job.php index dec88aeeab13..2738c1b49c17 100644 --- a/CRM/Admin/Form/Job.php +++ b/CRM/Admin/Form/Job.php @@ -101,7 +101,7 @@ public function buildQuickForm($check = FALSE) { $this->add('datepicker', 'scheduled_run_date', ts('Scheduled Run Date'), NULL, FALSE, ['minDate' => date('Y-m-d')]); $this->add('textarea', 'parameters', ts('Command parameters'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); // is this job active ? diff --git a/CRM/Admin/Form/MessageTemplates.php b/CRM/Admin/Form/MessageTemplates.php index 5f809f25bedd..47567b474274 100644 --- a/CRM/Admin/Form/MessageTemplates.php +++ b/CRM/Admin/Form/MessageTemplates.php @@ -179,7 +179,7 @@ public function buildQuickForm() { ) ) { $this->add('textarea', 'msg_html', ts('HTML Message'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); } else { @@ -194,7 +194,7 @@ public function buildQuickForm() { } $this->add('textarea', 'msg_text', ts('Text Message'), - "cols=50 rows=6" + ['cols' => 50, 'rows' => 6] ); $this->add('select', 'pdf_format_id', ts('PDF Page Format'), diff --git a/CRM/Admin/Form/PdfFormats.php b/CRM/Admin/Form/PdfFormats.php index 0ec9042d5839..1f7c10a0f50c 100644 --- a/CRM/Admin/Form/PdfFormats.php +++ b/CRM/Admin/Form/PdfFormats.php @@ -67,7 +67,7 @@ public function buildQuickForm() { ['onChange' => "selectPaper( this.value );"] ); - $this->add('static', 'paper_dimensions', NULL, ts('Width x Height')); + $this->add('static', 'paper_dimensions', ts('Width x Height')); $this->add('select', 'orientation', ts('Orientation'), CRM_Core_BAO_PdfFormat::getPageOrientations(), FALSE, ['onChange' => "updatePaperDimensions();"] ); diff --git a/CRM/Contact/Form/Task/EmailTrait.php b/CRM/Contact/Form/Task/EmailTrait.php index c426d8d4bb19..67c06172d8e1 100644 --- a/CRM/Contact/Form/Task/EmailTrait.php +++ b/CRM/Contact/Form/Task/EmailTrait.php @@ -247,7 +247,7 @@ public function buildQuickForm() { $this->assign('totalSelectedContacts', count($this->_contactIds)); - $this->add('text', 'subject', ts('Subject'), 'size=50 maxlength=254', TRUE); + $this->add('text', 'subject', ts('Subject'), ['size' => 50, 'maxlength' => 254], TRUE); $this->add('select', 'from_email_address', ts('From'), $this->_fromEmails, TRUE); diff --git a/CRM/Contribute/Form/ContributionPage/Premium.php b/CRM/Contribute/Form/ContributionPage/Premium.php index ba5b7863b777..7683f3d90015 100644 --- a/CRM/Contribute/Form/ContributionPage/Premium.php +++ b/CRM/Contribute/Form/ContributionPage/Premium.php @@ -52,7 +52,7 @@ public function buildQuickForm() { $this->addElement('text', 'premiums_intro_title', ts('Title'), $attributes['premiums_intro_title']); - $this->add('textarea', 'premiums_intro_text', ts('Introductory Message'), 'rows=5, cols=50'); + $this->add('textarea', 'premiums_intro_text', ts('Introductory Message'), ['cols' => 50, 'rows' => 5]); $this->add('text', 'premiums_contact_email', ts('Contact Email') . ' ', $attributes['premiums_contact_email']); diff --git a/CRM/Contribute/Form/ManagePremiums.php b/CRM/Contribute/Form/ManagePremiums.php index 61da18d8f443..3a4a97e0ec21 100644 --- a/CRM/Contribute/Form/ManagePremiums.php +++ b/CRM/Contribute/Form/ManagePremiums.php @@ -81,7 +81,7 @@ public function buildQuickForm() { ]); $this->add('text', 'sku', ts('SKU'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'sku')); - $this->add('textarea', 'description', ts('Description'), 'rows=3, cols=60'); + $this->add('textarea', 'description', ts('Description'), ['cols' => 60, 'rows' => 3]); $image['image'] = $this->createElement('radio', NULL, NULL, ts('Upload from my computer'), 'image', 'onclick="add_upload_file_block(\'image\');'); $image['thumbnail'] = $this->createElement('radio', NULL, NULL, ts('Display image and thumbnail from these locations on the web:'), 'thumbnail', 'onclick="add_upload_file_block(\'thumbnail\');'); @@ -94,7 +94,7 @@ public function buildQuickForm() { $this->addElement('text', 'imageUrl', ts('Image URL')); $this->addElement('text', 'thumbnailUrl', ts('Thumbnail URL')); - $this->add('file', 'uploadFile', ts('Image File Name'), 'onChange="select_option();"'); + $this->add('file', 'uploadFile', ts('Image File Name'), ['onChange' => 'select_option();']); $this->add('text', 'price', ts('Market Value'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'price'), TRUE); $this->addRule('price', ts('Please enter the Market Value for this product.'), 'money'); @@ -105,7 +105,7 @@ public function buildQuickForm() { $this->add('text', 'min_contribution', ts('Minimum Contribution Amount'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'min_contribution'), TRUE); $this->addRule('min_contribution', ts('Please enter a monetary value for the Minimum Contribution Amount.'), 'money'); - $this->add('textarea', 'options', ts('Options'), 'rows=3, cols=60'); + $this->add('textarea', 'options', ts('Options'), ['cols' => 60, 'rows' => 3]); $this->add('select', 'period_type', ts('Period Type'), [ '' => '- select -', diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 41cc7fe7ba88..030c88b17c94 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -667,11 +667,13 @@ public static function addQuickFormElement( $element = NULL; $customFieldAttributes = []; + // DAO stores attributes as a string, but it's hard to manipulate and + // CRM_Core_Form::add() wants them as an array. + $fieldAttributes = self::attributesFromString($field->attributes); + // Custom field HTML should indicate group+field name $groupName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id); - $dataCrmCustomVal = $groupName . ':' . $field->name; - $dataCrmCustomAttr = 'data-crm-custom="' . $dataCrmCustomVal . '"'; - $field->attributes .= $dataCrmCustomAttr; + $fieldAttributes['data-crm-custom'] = $groupName . ':' . $field->name; // Fixed for Issue CRM-2183 if ($widget == 'TextArea' && $search) { @@ -680,39 +682,34 @@ public static function addQuickFormElement( $placeholder = $search ? ts('- any -') : ($useRequired ? ts('- select -') : ts('- none -')); - $isSelect = (in_array($widget, [ + if (in_array($widget, [ 'Select', 'CheckBox', 'Radio', - ])); - - if ($isSelect) { + ])) { $options = $field->getOptions($search ? 'search' : 'create'); // Consolidate widget types to simplify the below switch statement - if ($search || (strpos($widget, 'Select') !== FALSE)) { + if ($search) { $widget = 'Select'; } - $customFieldAttributes['data-crm-custom'] = $dataCrmCustomVal; - $selectAttributes = ['class' => 'crm-select2']; - // Search field is always multi-select if ($search || (self::isSerialized($field) && $widget === 'Select')) { - $selectAttributes['class'] .= ' huge'; - $selectAttributes['multiple'] = 'multiple'; - $selectAttributes['placeholder'] = $placeholder; + $fieldAttributes['class'] .= ltrim($fieldAttributes['class'] ?? '' . ' huge'); + $fieldAttributes['multiple'] = 'multiple'; + $fieldAttributes['placeholder'] = $placeholder; } // Add data for popup link. Normally this is handled by CRM_Core_Form->addSelect $canEditOptions = CRM_Core_Permission::check('administer CiviCRM'); - if ($field->option_group_id && !$search && $isSelect && $canEditOptions) { + if ($field->option_group_id && !$search && $canEditOptions) { $customFieldAttributes += [ 'data-api-entity' => $field->getEntity(), 'data-api-field' => 'custom_' . $field->id, 'data-option-edit-path' => 'civicrm/admin/options/' . CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', $field->option_group_id), ]; - $selectAttributes += $customFieldAttributes; + $fieldAttributes = array_merge($fieldAttributes, $customFieldAttributes); } } @@ -728,50 +725,39 @@ public static function addQuickFormElement( case 'Text': case 'Link': if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { if ($field->text_length) { - $field->attributes .= ' maxlength=' . $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; if ($field->text_length < 20) { - $field->attributes .= ' size=' . $field->text_length; + $fieldAttributes['size'] = $field->text_length; } } $element = $qf->add('text', $elementName, $label, - $field->attributes, + $fieldAttributes, $useRequired && !$search ); } break; case 'TextArea': - $attributes = $dataCrmCustomAttr; - if ($field->note_rows) { - $attributes .= 'rows=' . $field->note_rows; - } - else { - $attributes .= 'rows=4'; - } - if ($field->note_columns) { - $attributes .= ' cols=' . $field->note_columns; - } - else { - $attributes .= ' cols=60'; - } + $fieldAttributes['rows'] = $field->note_rows ?? 4; + $fieldAttributes['cols'] = $field->note_columns ?? 60; + if ($field->text_length) { - $attributes .= ' maxlength=' . $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; } $element = $qf->add('textarea', $elementName, $label, - $attributes, + $fieldAttributes, $useRequired && !$search ); break; case 'Select Date': - $attr = ['data-crm-custom' => $dataCrmCustomVal]; //CRM-18379: Fix for date range of 'Select Date' custom field when include in profile. $minYear = isset($field->start_date_years) ? (date('Y') - $field->start_date_years) : NULL; $maxYear = isset($field->end_date_years) ? (date('Y') + $field->end_date_years) : NULL; @@ -784,40 +770,40 @@ public static function addQuickFormElement( 'time' => $field->time_format ? $field->time_format * 12 : FALSE, ]; if ($field->is_search_range && $search) { - $qf->add('datepicker', $elementName . '_from', $label, $attr + array('placeholder' => ts('From')), FALSE, $params); - $qf->add('datepicker', $elementName . '_to', NULL, $attr + array('placeholder' => ts('To')), FALSE, $params); + $qf->add('datepicker', $elementName . '_from', $label, $fieldAttributes + array('placeholder' => ts('From')), FALSE, $params); + $qf->add('datepicker', $elementName . '_to', NULL, $fieldAttributes + array('placeholder' => ts('To')), FALSE, $params); } else { - $element = $qf->add('datepicker', $elementName, $label, $attr, $useRequired && !$search, $params); + $element = $qf->add('datepicker', $elementName, $label, $fieldAttributes, $useRequired && !$search, $params); } break; case 'Radio': if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { - parse_str($field->attributes, $radioAttributes); - $radioAttributes = array_merge($radioAttributes, $customFieldAttributes); + $fieldAttributes = array_merge($fieldAttributes, $customFieldAttributes); if ($search || empty($useRequired)) { - $radioAttributes['allowClear'] = TRUE; + $fieldAttributes['allowClear'] = TRUE; } - $qf->addRadio($elementName, $label, $options, $radioAttributes, NULL, $useRequired); + $qf->addRadio($elementName, $label, $options, $fieldAttributes, NULL, $useRequired); } break; // For all select elements case 'Select': + $fieldAttributes['class'] .= ltrim($fieldAttributes['class'] ?? '' . ' crm-select2'); if ($field->is_search_range && $search && in_array($field->data_type, $rangeDataTypes)) { - $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $field->attributes); - $qf->add('text', $elementName . '_to', ts('To'), $field->attributes); + $qf->add('text', $elementName . '_from', $label . ' ' . ts('From'), $fieldAttributes); + $qf->add('text', $elementName . '_to', ts('To'), $fieldAttributes); } else { - if (empty($selectAttributes['multiple'])) { + if (empty($fieldAttributes['multiple'])) { $options = ['' => $placeholder] + $options; } - $element = $qf->add('select', $elementName, $label, $options, $useRequired && !$search, $selectAttributes); + $element = $qf->add('select', $elementName, $label, $options, $useRequired && !$search, $fieldAttributes); // Add and/or option for fields that store multiple values if ($search && self::isSerialized($field)) { @@ -836,6 +822,9 @@ public static function addQuickFormElement( case 'CheckBox': $check = []; foreach ($options as $v => $l) { + // TODO: I'm not sure if this is supposed to exclude whatever might be + // in $field->attributes (available in array format as + // $fieldAttributes). Leaving as-is for now. $check[] = &$qf->addElement('advcheckbox', $v, NULL, $l, $customFieldAttributes); } @@ -859,44 +848,31 @@ public static function addQuickFormElement( strtolower($field->html_type), $elementName, $label, - $field->attributes, + $fieldAttributes, $useRequired && !$search ); $qf->addUploadElement($elementName); break; case 'RichTextEditor': - $attributes = [ - 'rows' => $field->note_rows, - 'cols' => $field->note_columns, - 'data-crm-custom' => $dataCrmCustomVal, - ]; + $fieldAttributes['rows'] = $field->note_rows; + $fieldAttributes['cols'] = $field->note_columns; if ($field->text_length) { - $attributes['maxlength'] = $field->text_length; + $fieldAttributes['maxlength'] = $field->text_length; } - $element = $qf->add('wysiwyg', $elementName, $label, $attributes, $useRequired && !$search); + $element = $qf->add('wysiwyg', $elementName, $label, $fieldAttributes, $useRequired && !$search); break; case 'Autocomplete-Select': static $customUrls = []; - // Fixme: why is this a string in the first place?? - $attributes = []; - if ($field->attributes) { - foreach (explode(' ', $field->attributes) as $at) { - if (strpos($at, '=')) { - list($k, $v) = explode('=', $at); - $attributes[$k] = trim($v, ' "'); - } - } - } if ($field->data_type == 'ContactReference') { // break if contact does not have permission to access ContactReference if (!CRM_Core_Permission::check('access contact reference fields')) { break; } - $attributes['class'] = (isset($attributes['class']) ? $attributes['class'] . ' ' : '') . 'crm-form-contact-reference huge'; - $attributes['data-api-entity'] = 'Contact'; - $element = $qf->add('text', $elementName, $label, $attributes, $useRequired && !$search); + $fieldAttributes['class'] = ltrim($fieldAttributes['class'] ?? '' . ' crm-form-contact-reference huge'); + $fieldAttributes['data-api-entity'] = 'Contact'; + $element = $qf->add('text', $elementName, $label, $fieldAttributes, $useRequired && !$search); $urlParams = "context=customfield&id={$field->id}"; $idOfelement = $elementName; @@ -915,7 +891,7 @@ public static function addQuickFormElement( } else { // FIXME: This won't work with customFieldOptions hook - $attributes += [ + $fieldAttributes += [ 'entity' => 'OptionValue', 'placeholder' => $placeholder, 'multiple' => $search, @@ -923,7 +899,7 @@ public static function addQuickFormElement( 'params' => ['option_group_id' => $field->option_group_id, 'is_active' => 1], ], ]; - $element = $qf->addEntityRef($elementName, $label, $attributes, $useRequired && !$search); + $element = $qf->addEntityRef($elementName, $label, $fieldAttributes, $useRequired && !$search); } $qf->assign('customUrls', $customUrls); @@ -973,6 +949,27 @@ public static function addQuickFormElement( return $element; } + /** + * Take a string of HTML element attributes and turn it into an associative + * array. + * + * @param string $attrString + * The attributes as a string, e.g. `rows=3 cols=40`. + * + * @return array + * The attributes as an array, e.g. `['rows' => 3, 'cols' => 40]`. + */ + public static function attributesFromString($attrString) { + $attributes = []; + foreach (explode(' ', $attrString) as $at) { + if (strpos($at, '=')) { + list($k, $v) = explode('=', $at); + $attributes[$k] = trim($v, ' "'); + } + } + return $attributes; + } + /** * Delete the Custom Field. * diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index fe06bb991528..f149bdecf31d 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -350,7 +350,7 @@ public function registerRules() { * @param string $type * @param string $name * @param string $label - * @param string|array $attributes (options for select elements) + * @param array $attributes (options for select elements) * @param bool $required * @param array $extra * (attributes for select elements). @@ -364,11 +364,18 @@ public function registerRules() { */ public function &add( $type, $name, $label = '', - $attributes = '', $required = FALSE, $extra = NULL + $attributes = NULL, $required = FALSE, $extra = NULL ) { if ($type === 'radio') { CRM_Core_Error::deprecatedFunctionWarning('CRM_Core_Form::addRadio'); } + + if ($attributes && !is_array($attributes)) { + // The $attributes param used to allow for strings and would default to an + // empty string. However, now that the variable is heavily manipulated, + // we should expect it to always be an array. + Civi::log()->warning('Attributes passed to CRM_Core_Form::add() are not an array.', ['civi.tag' => 'deprecated']); + } // Fudge some extra types that quickform doesn't support $inputType = $type; if ($type == 'wysiwyg' || in_array($type, self::$html5Types)) { diff --git a/CRM/Import/DataSource/SQL.php b/CRM/Import/DataSource/SQL.php index 26d3e09faa1a..c1cbb61e1e4d 100644 --- a/CRM/Import/DataSource/SQL.php +++ b/CRM/Import/DataSource/SQL.php @@ -49,7 +49,7 @@ public function preProcess(&$form) { */ public function buildQuickForm(&$form) { $form->add('hidden', 'hidden_dataSource', 'CRM_Import_DataSource_SQL'); - $form->add('textarea', 'sqlQuery', ts('Specify SQL Query'), 'rows=10 cols=45', TRUE); + $form->add('textarea', 'sqlQuery', ts('Specify SQL Query'), ['rows' => 10, 'cols' => 45], TRUE); $form->addFormRule(['CRM_Import_DataSource_SQL', 'formRule'], $form); } diff --git a/CRM/Price/Form/Field.php b/CRM/Price/Form/Field.php index a7f7420f9a14..96aaf3b55854 100644 --- a/CRM/Price/Form/Field.php +++ b/CRM/Price/Form/Field.php @@ -181,7 +181,7 @@ public function buildQuickForm() { $this->add('text', 'label', ts('Field Label'), CRM_Core_DAO::getAttribute('CRM_Price_DAO_PriceField', 'label'), TRUE); // html_type - $javascript = 'onchange="option_html_type(this.form)";'; + $javascript = ['onchange' => 'option_html_type(this.form);']; $htmlTypes = CRM_Price_BAO_PriceField::htmlTypes(); diff --git a/CRM/SMS/Form/Provider.php b/CRM/SMS/Form/Provider.php index 68e64de811b8..c78f55fcd4e5 100644 --- a/CRM/SMS/Form/Provider.php +++ b/CRM/SMS/Form/Provider.php @@ -94,7 +94,7 @@ public function buildQuickForm() { $this->add('text', 'api_url', ts('API Url'), $attributes['api_url'], TRUE); $this->add('textarea', 'api_params', ts('API Parameters'), - "cols=50 rows=6", TRUE + ['cols' => 50, 'rows' => 6], TRUE ); $this->add('checkbox', 'is_active', ts('Is this provider active?')); From 9ca69805338baab6882b8d1cc887903f4b16eceb Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Sat, 20 Jun 2020 14:00:33 -0400 Subject: [PATCH 2/4] Cleanup with ?? --- CRM/Core/Form.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index f149bdecf31d..f15287eee95c 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -420,7 +420,7 @@ public function &add( $attributes['data-crm-datepicker'] = json_encode((array) $extra); if (!empty($attributes['aria-label']) || $label) { - $attributes['aria-label'] = CRM_Utils_Array::value('aria-label', $attributes, $label); + $attributes['aria-label'] = $attributes['aria-label'] ?? $label; } $type = "text"; } @@ -1211,12 +1211,7 @@ public function &addRadio($name, $title, $values, $attributes = [], $separator = $optAttributes = $attributes; if (!empty($optionAttributes[$key])) { foreach ($optionAttributes[$key] as $optAttr => $optVal) { - if (!empty($optAttributes[$optAttr])) { - $optAttributes[$optAttr] .= ' ' . $optVal; - } - else { - $optAttributes[$optAttr] = $optVal; - } + $optAttributes[$optAttr] = ltrim(($optAttributes[$optAttr] ?? '') . ' ' . $optVal); } } // We use a class here to avoid html5 issues with collapsed cutsomfield sets. @@ -2067,7 +2062,7 @@ public function addEntityRef($name, $label = '', $props = [], $required = FALSE) // Default properties $props['api'] = CRM_Utils_Array::value('api', $props, []); $props['entity'] = CRM_Core_DAO_AllCoreTables::convertEntityNameToCamel(CRM_Utils_Array::value('entity', $props, 'Contact')); - $props['class'] = ltrim(CRM_Utils_Array::value('class', $props, '') . ' crm-form-entityref'); + $props['class'] = ltrim(($props['class'] ?? '') . ' crm-form-entityref'); if (array_key_exists('create', $props) && empty($props['create'])) { unset($props['create']); From a2eb61520b685cf139a829d65015b2579caed27d Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Mon, 14 Sep 2020 14:38:27 -0400 Subject: [PATCH 3/4] Select placeholders should always say what kind of thing they select --- CRM/ACL/Form/ACL.php | 13 ++--- CRM/ACL/Form/EntityRole.php | 3 +- CRM/Activity/BAO/Query.php | 2 +- CRM/Activity/Form/Activity.php | 6 ++- CRM/Contribute/Form/ManagePremiums.php | 11 ++-- CRM/Core/BAO/CustomField.php | 10 ++-- CRM/Core/BAO/UFGroup.php | 71 ++++++++++---------------- CRM/Core/Form.php | 47 +++++++++++++---- CRM/SMS/Form/Provider.php | 2 +- 9 files changed, 89 insertions(+), 76 deletions(-) diff --git a/CRM/ACL/Form/ACL.php b/CRM/ACL/Form/ACL.php index de1665dcd466..1c7af2a0952f 100644 --- a/CRM/ACL/Form/ACL.php +++ b/CRM/ACL/Form/ACL.php @@ -96,11 +96,12 @@ public function buildQuickForm() { $this->add('text', 'name', ts('Description'), CRM_Core_DAO::getAttribute('CRM_ACL_DAO_ACL', 'name'), TRUE); - $operations = ['' => ts('- select -')] + CRM_ACL_BAO_ACL::operation(); $this->add('select', 'operation', ts('Operation'), - $operations, TRUE + CRM_ACL_BAO_ACL::operation(), + TRUE, + ['placeholder' => TRUE] ); $objTypes = [ @@ -129,22 +130,22 @@ public function buildQuickForm() { $this->add('select', 'entity_id', $label, $role, TRUE); $group = [ - '-1' => ts('- select -'), + '-1' => ts('- select group -'), '0' => ts('All Groups'), ] + CRM_Core_PseudoConstant::group(); $customGroup = [ - '-1' => ts('- select -'), + '-1' => ts('- select set of custom fields -'), '0' => ts('All Custom Groups'), ] + CRM_Core_PseudoConstant::get('CRM_Core_DAO_CustomField', 'custom_group_id'); $ufGroup = [ - '-1' => ts('- select -'), + '-1' => ts('- select profile -'), '0' => ts('All Profiles'), ] + CRM_Core_PseudoConstant::get('CRM_Core_DAO_UFField', 'uf_group_id'); $event = [ - '-1' => ts('- select -'), + '-1' => ts('- select event -'), '0' => ts('All Events'), ] + CRM_Event_PseudoConstant::event(NULL, FALSE, "( is_template IS NULL OR is_template != 1 )"); diff --git a/CRM/ACL/Form/EntityRole.php b/CRM/ACL/Form/EntityRole.php index d12686fe433e..7d599deff475 100644 --- a/CRM/ACL/Form/EntityRole.php +++ b/CRM/ACL/Form/EntityRole.php @@ -26,9 +26,8 @@ public function buildQuickForm() { return; } - $aclRoles = ['' => ts('- select -')] + CRM_Core_OptionGroup::values('acl_role'); $this->add('select', 'acl_role_id', ts('ACL Role'), - $aclRoles, TRUE + CRM_Core_OptionGroup::values('acl_role'), TRUE, ['placeholder' => TRUE] ); $label = ts('Assigned to'); diff --git a/CRM/Activity/BAO/Query.php b/CRM/Activity/BAO/Query.php index d432467e8d40..0aa2df666a3f 100644 --- a/CRM/Activity/BAO/Query.php +++ b/CRM/Activity/BAO/Query.php @@ -478,7 +478,7 @@ public static function buildSearchForm(&$form) { 'multiple' => 'multiple', 'class' => 'crm-select2', - 'placeholder' => ts('- select -'), + 'placeholder' => ts('- select tags -'), ] ); } diff --git a/CRM/Activity/Form/Activity.php b/CRM/Activity/Form/Activity.php index 4d457aafda3a..c6b07c4a397d 100644 --- a/CRM/Activity/Form/Activity.php +++ b/CRM/Activity/Form/Activity.php @@ -628,10 +628,11 @@ public function buildQuickForm() { $this->assign('suppressForm', FALSE); $element = $this->add('select', 'activity_type_id', ts('Activity Type'), - ['' => '- ' . ts('select') . ' -'] + $this->_fields['followup_activity_type_id']['attributes'], + $this->_fields['followup_activity_type_id']['attributes'], FALSE, [ 'onchange' => "CRM.buildCustomData( 'Activity', this.value, false, false, false, false, false, false, {$this->_currentlyViewedContactId});", 'class' => 'crm-select2 required', + 'placeholder' => TRUE, ] ); @@ -695,7 +696,8 @@ public function buildQuickForm() { $responseOptions = CRM_Campaign_BAO_Survey::getResponsesOptions($surveyId); if ($responseOptions) { $this->add('select', 'result', ts('Result'), - ['' => ts('- select -')] + array_combine($responseOptions, $responseOptions) + array_combine($responseOptions, $responseOptions), + FALSE, ['placeholder' => TRUE] ); } $surveyTitle = NULL; diff --git a/CRM/Contribute/Form/ManagePremiums.php b/CRM/Contribute/Form/ManagePremiums.php index 3a4a97e0ec21..85b8ad03c9fb 100644 --- a/CRM/Contribute/Form/ManagePremiums.php +++ b/CRM/Contribute/Form/ManagePremiums.php @@ -108,18 +108,17 @@ public function buildQuickForm() { $this->add('textarea', 'options', ts('Options'), ['cols' => 60, 'rows' => 3]); $this->add('select', 'period_type', ts('Period Type'), [ - '' => '- select -', 'rolling' => 'Rolling', 'fixed' => 'Fixed', - ]); + ], FALSE, ['placeholder' => TRUE]); $this->add('text', 'fixed_period_start_day', ts('Fixed Period Start Day'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'fixed_period_start_day')); - $this->add('Select', 'duration_unit', ts('Duration Unit'), ['' => '- select period -'] + CRM_Core_SelectValues::getPremiumUnits()); + $this->add('Select', 'duration_unit', ts('Duration Unit'), CRM_Core_SelectValues::getPremiumUnits(), FALSE, ['placeholder' => ts('- select period -')]); $this->add('text', 'duration_interval', ts('Duration'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'duration_interval')); - $this->add('Select', 'frequency_unit', ts('Frequency Unit'), ['' => '- select period -'] + CRM_Core_SelectValues::getPremiumUnits()); + $this->add('Select', 'frequency_unit', ts('Frequency Unit'), CRM_Core_SelectValues::getPremiumUnits(), FALSE, ['placeholder' => ts('- select period -')]); $this->add('text', 'frequency_interval', ts('Frequency'), CRM_Core_DAO::getAttribute('CRM_Contribute_DAO_Product', 'frequency_interval')); @@ -157,7 +156,9 @@ public function buildQuickForm() { 'select', 'financial_type_id', ts('Financial Type'), - ['' => ts('- select -')] + $financialType + $financialType, + FALSE, + ['placeholder' => TRUE] ); $this->add('checkbox', 'is_active', ts('Enabled?')); diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 030c88b17c94..a8614578e2b2 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -667,6 +667,10 @@ public static function addQuickFormElement( $element = NULL; $customFieldAttributes = []; + if (!isset($label)) { + $label = $field->label; + } + // DAO stores attributes as a string, but it's hard to manipulate and // CRM_Core_Form::add() wants them as an array. $fieldAttributes = self::attributesFromString($field->attributes); @@ -680,7 +684,7 @@ public static function addQuickFormElement( $widget = 'Text'; } - $placeholder = $search ? ts('- any -') : ($useRequired ? ts('- select -') : ts('- none -')); + $placeholder = $search ? ts('- any %1 -', [1 => $label]) : ts('- select %1 -', [1 => $label]); if (in_array($widget, [ 'Select', @@ -715,10 +719,6 @@ public static function addQuickFormElement( $rangeDataTypes = ['Int', 'Float', 'Money']; - if (!isset($label)) { - $label = $field->label; - } - // at some point in time we might want to split the below into small functions switch ($widget) { diff --git a/CRM/Core/BAO/UFGroup.php b/CRM/Core/BAO/UFGroup.php index 88d84f04afa8..543560b96958 100644 --- a/CRM/Core/BAO/UFGroup.php +++ b/CRM/Core/BAO/UFGroup.php @@ -1869,7 +1869,7 @@ public static function buildProfile( } } elseif (substr($fieldName, 0, 7) === 'country') { - $form->add('select', $name, $title, ['' => ts('- select -')] + CRM_Core_PseudoConstant::country(), $required, $selectAttributes); + $form->add('select', $name, $title, CRM_Core_PseudoConstant::country(), $required, $selectAttributes); $config = CRM_Core_Config::singleton(); if (!in_array($mode, [CRM_Profile_Form::MODE_EDIT, CRM_Profile_Form::MODE_SEARCH]) && $config->defaultContactCountry @@ -1895,16 +1895,12 @@ public static function buildProfile( $providerName = substr($name, 0, -1) . '-provider_id]'; } $form->add('select', $providerName, NULL, - [ - '' => ts('- select -'), - ] + CRM_Core_PseudoConstant::get('CRM_Core_DAO_IM', 'provider_id'), $required + CRM_Core_PseudoConstant::get('CRM_Core_DAO_IM', 'provider_id'), $required ); } else { $form->add('select', $name . '-provider_id', $title, - [ - '' => ts('- select -'), - ] + CRM_Core_PseudoConstant::get('CRM_Core_DAO_IM', 'provider_id'), $required + CRM_Core_PseudoConstant::get('CRM_Core_DAO_IM', 'provider_id'), $required ); } @@ -1916,7 +1912,7 @@ public static function buildProfile( elseif (CRM_Utils_Array::value('name', $field) == 'membership_type') { list($orgInfo, $types) = CRM_Member_BAO_MembershipType::getMembershipTypeInfo(); $sel = &$form->addElement('hierselect', $name, $title); - $select = ['' => ts('- select -')]; + $select = ['' => ts('- select membership type -')]; if (count($orgInfo) == 1 && $field['is_required']) { // we only have one org - so we should default to it. Not sure about defaulting to first type // as it could be missed - so adding a select @@ -1932,9 +1928,7 @@ public static function buildProfile( } elseif (CRM_Utils_Array::value('name', $field) == 'membership_status') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Member_PseudoConstant::membershipStatus(NULL, NULL, 'label'), $required + CRM_Member_PseudoConstant::membershipStatus(NULL, NULL, 'label'), $required ); } elseif (in_array($fieldName, ['gender_id', 'communication_style_id'])) { @@ -1999,7 +1993,7 @@ public static function buildProfile( 'contact_type' => $profileType, 'greeting_type' => $fieldName, ]; - $form->add('select', $name, $title, ['' => ts('- select -')] + CRM_Core_PseudoConstant::greeting($greeting), $required); + $form->add('select', $name, $title, CRM_Core_PseudoConstant::greeting($greeting), $required, ['placeholder' => TRUE]); // add custom greeting element $form->add('text', $fieldName . '_custom', ts('Custom %1', [1 => ucwords(str_replace('_', ' ', $fieldName))]), NULL, FALSE @@ -2019,7 +2013,7 @@ public static function buildProfile( $form->add('select', $name, $title, CRM_Core_SelectValues::pmf()); } elseif ($fieldName === 'preferred_language') { - $form->add('select', $name, $title, ['' => ts('- select -')] + CRM_Contact_BAO_Contact::buildOptions('preferred_language'), $required); + $form->add('select', $name, $title, CRM_Contact_BAO_Contact::buildOptions('preferred_language'), $required, ['placeholder' => TRUE]); } elseif ($fieldName == 'external_identifier') { $form->add('text', $name, $title, $attributes, $required); @@ -2078,35 +2072,29 @@ public static function buildProfile( elseif ($fieldName === 'product_name') { list($products, $options) = CRM_Contribute_BAO_Premium::getPremiumProductInfo(); $sel = &$form->addElement('hierselect', $name, $title); - $products = ['0' => ts('- select -')] + $products; + $products = ['0' => ts('- select %1 -', [1 => $title])] + $products; $sel->setOptions([$products, $options]); } elseif ($fieldName === 'payment_instrument') { $form->add('select', $name, $title, - ['' => ts('- select -')] + CRM_Contribute_PseudoConstant::paymentInstrument(), $required); + CRM_Contribute_PseudoConstant::paymentInstrument(), $required, ['placeholder' => TRUE]); } elseif ($fieldName === 'financial_type') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Contribute_PseudoConstant::financialType(), $required + CRM_Contribute_PseudoConstant::financialType(), $required, ['placeholder' => TRUE] ); } elseif ($fieldName === 'contribution_status_id') { $contributionStatuses = CRM_Contribute_BAO_Contribution_Utils::getContributionStatuses(); $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + $contributionStatuses, $required + $contributionStatuses, $required, ['placeholder' => TRUE] ); } elseif ($fieldName === 'soft_credit_type') { $name = "soft_credit_type[$rowNumber]"; $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Core_OptionGroup::values("soft_credit_type") + CRM_Core_OptionGroup::values("soft_credit_type"), ['placeholder' => TRUE] ); //CRM-15350: choose SCT field default value as 'Gift' for membership use //else (for contribution), use configured SCT default value @@ -2124,23 +2112,20 @@ public static function buildProfile( } elseif ($fieldName == 'contribution_page_id') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Contribute_PseudoConstant::contributionPage(), $required, 'class="big"' + CRM_Contribute_PseudoConstant::contributionPage(), $required, [ + 'class' => 'big', + 'placeholder' => TRUE, + ] ); } elseif ($fieldName == 'activity_status_id') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Core_PseudoConstant::activityStatus(), $required + CRM_Core_PseudoConstant::activityStatus(), $required, ['placeholder' => TRUE] ); } elseif ($fieldName == 'activity_engagement_level') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Campaign_PseudoConstant::engagementLevel(), $required + CRM_Campaign_PseudoConstant::engagementLevel(), $required, ['placeholder' => TRUE] ); } elseif ($fieldName == 'participant_status') { @@ -2149,9 +2134,7 @@ public static function buildProfile( $cond = 'visibility_id = 1'; } $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Event_PseudoConstant::participantStatus(NULL, $cond, 'label'), $required + CRM_Event_PseudoConstant::participantStatus(NULL, $cond, 'label'), $required, ['placeholder' => TRUE] ); } elseif ($fieldName == 'participant_role') { @@ -2160,9 +2143,7 @@ public static function buildProfile( } else { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Event_PseudoConstant::participantRole(), $required + CRM_Event_PseudoConstant::participantRole(), $required, ['placeholder' => TRUE] ); } } @@ -2181,9 +2162,11 @@ public static function buildProfile( $form->_componentCampaigns )); $form->add('select', $name, $title, + $campaigns, $required, [ - '' => ts('- select -'), - ] + $campaigns, $required, 'class="crm-select2 big"' + 'class' => 'crm-select2 big', + 'placeholder' => TRUE, + ] ); } } @@ -2196,10 +2179,8 @@ public static function buildProfile( } elseif ($fieldName == 'case_status') { $form->add('select', $name, $title, - [ - '' => ts('- select -'), - ] + CRM_Case_BAO_Case::buildOptions('case_status_id', 'create'), - $required + CRM_Case_BAO_Case::buildOptions('case_status_id', 'create'), + $required, ['placeholder' => TRUE] ); } else { diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index f15287eee95c..2cd2c226b5db 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -436,7 +436,7 @@ public function &add( // Add placeholder option for select if (isset($extra['placeholder'])) { if ($extra['placeholder'] === TRUE) { - $extra['placeholder'] = $required ? ts('- select -') : ts('- none -'); + $extra['placeholder'] = ts('- select %1 -', [1 => $label]); } if (($extra['placeholder'] || $extra['placeholder'] === '') && empty($extra['multiple']) && is_array($attributes) && !isset($attributes[''])) { $attributes = ['' => $extra['placeholder']] + $attributes; @@ -1498,8 +1498,8 @@ public function addSelect($name, $props = [], $required = FALSE) { $info = civicrm_api3($props['entity'], 'getoptions', $props); $options = $info['values']; } - if (!array_key_exists('placeholder', $props)) { - $props['placeholder'] = $required ? ts('- select -') : (CRM_Utils_Array::value('context', $props) == 'search' ? ts('- any -') : ts('- none -')); + if (!array_key_exists('placeholder', $props) && $placeholder = self::selectOrAnyPlaceholder($props, $required)) { + $props['placeholder'] = $placeholder; } // Handle custom field if (strpos($name, 'custom_') === 0 && is_numeric($name[7])) { @@ -1534,6 +1534,34 @@ public function addSelect($name, $props = [], $required = FALSE) { return $this->add('select', $name, $label, $options, $required, $props); } + /** + * Handles a repeated bit supplying a placeholder for entity selection + * + * @param string $props + * The field properties, including the entity and context. + * @param bool $required + * If the field is required. + * @return string + * The placeholder text. + */ + private static function selectOrAnyPlaceholder($props, $required) { + if (empty($props['entity'])) { + return NULL; + } + $daoToClass = CRM_Core_DAO_AllCoreTables::daoToClass(); + if (array_key_exists($props['entity'], $daoToClass)) { + $daoClass = $daoToClass[$props['entity']]; + $tsPlaceholder = $daoClass::getEntityTitle(); + } + else { + $tsPlaceholder = ts('option'); + } + if (($props['context'] ?? '') == 'search' && !$required) { + return ts('- any %1 -', [1 => $tsPlaceholder]); + } + return ts('- select %1 -', [1 => $tsPlaceholder]); + } + /** * Adds a field based on metadata. * @@ -1690,8 +1718,8 @@ public function addField($name, $props = [], $required = FALSE, $legacyDate = TR case 'Select': case 'Select2': $props['class'] = CRM_Utils_Array::value('class', $props, 'big') . ' crm-select2'; - if (!array_key_exists('placeholder', $props)) { - $props['placeholder'] = $required ? ts('- select -') : ($context == 'search' ? ts('- any -') : ts('- none -')); + if (!array_key_exists('placeholder', $props) && $placeholder = self::selectOrAnyPlaceholder($props, $required)) { + $props['placeholder'] = $placeholder; } // TODO: Add and/or option for fields that store multiple values return $this->add(strtolower($widget), $name, $label, $options, $required, $props); @@ -2061,14 +2089,14 @@ public function addCurrency( public function addEntityRef($name, $label = '', $props = [], $required = FALSE) { // Default properties $props['api'] = CRM_Utils_Array::value('api', $props, []); - $props['entity'] = CRM_Core_DAO_AllCoreTables::convertEntityNameToCamel(CRM_Utils_Array::value('entity', $props, 'Contact')); + $props['entity'] = CRM_Core_DAO_AllCoreTables::convertEntityNameToCamel($props['entity'] ?? 'Contact'); $props['class'] = ltrim(($props['class'] ?? '') . ' crm-form-entityref'); if (array_key_exists('create', $props) && empty($props['create'])) { unset($props['create']); } - $props['placeholder'] = CRM_Utils_Array::value('placeholder', $props, $required ? ts('- select %1 -', [1 => ts(str_replace('_', ' ', $props['entity']))]) : ts('- none -')); + $props['placeholder'] = $props['placeholder'] ?? self::selectOrAnyPlaceholder($props, $required); $defaults = []; if (!empty($props['multiple'])) { @@ -2400,6 +2428,7 @@ public function setPageTitle($entityLabel) { * @return HTML_QuickForm_Element */ public function addChainSelect($elementName, $settings = []) { + $label = strpos($elementName, 'rovince') ? CRM_Core_DAO_StateProvince::getEntityTitle() : CRM_Core_DAO_County::getEntityTitle(); $props = $settings += [ 'control_field' => str_replace(['state_province', 'StateProvince', 'county', 'County'], [ 'country', @@ -2408,12 +2437,12 @@ public function addChainSelect($elementName, $settings = []) { 'StateProvince', ], $elementName), 'data-callback' => strpos($elementName, 'rovince') ? 'civicrm/ajax/jqState' : 'civicrm/ajax/jqCounty', - 'label' => strpos($elementName, 'rovince') ? ts('State/Province') : ts('County'), + 'label' => $label, 'data-empty-prompt' => strpos($elementName, 'rovince') ? ts('Choose country first') : ts('Choose state first'), 'data-none-prompt' => ts('- N/A -'), 'multiple' => FALSE, 'required' => FALSE, - 'placeholder' => empty($settings['required']) ? ts('- none -') : ts('- select -'), + 'placeholder' => ts('- select %1 -', [1 => $label]), ]; CRM_Utils_Array::remove($props, 'label', 'required', 'control_field', 'context'); $props['class'] = (empty($props['class']) ? '' : "{$props['class']} ") . 'crm-select2'; diff --git a/CRM/SMS/Form/Provider.php b/CRM/SMS/Form/Provider.php index c78f55fcd4e5..a9daf14057a7 100644 --- a/CRM/SMS/Form/Provider.php +++ b/CRM/SMS/Form/Provider.php @@ -70,7 +70,7 @@ public function buildQuickForm() { $providerNames = CRM_Core_OptionGroup::values('sms_provider_name', FALSE, FALSE, FALSE, NULL, 'label'); $apiTypes = CRM_Core_OptionGroup::values('sms_api_type', FALSE, FALSE, FALSE, NULL, 'label'); - $this->add('select', 'name', ts('Name'), ['' => '- select -'] + $providerNames, TRUE); + $this->add('select', 'name', ts('Name'), $providerNames, TRUE, ['placeholder' => TRUE]); $this->add('text', 'title', ts('Title'), $attributes['title'], TRUE From 5ad66c4f15cb85b49056337a474dc95236c1b83a Mon Sep 17 00:00:00 2001 From: Andrew Hunt Date: Mon, 14 Sep 2020 14:39:45 -0400 Subject: [PATCH 4/4] StateProvince DAO title should be State/Province --- CRM/Core/DAO/StateProvince.php | 4 ++-- xml/schema/Core/StateProvince.xml | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CRM/Core/DAO/StateProvince.php b/CRM/Core/DAO/StateProvince.php index 7342cb6e69a6..e376f57c6555 100644 --- a/CRM/Core/DAO/StateProvince.php +++ b/CRM/Core/DAO/StateProvince.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/StateProvince.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:ca396996ffd11fc0b89e34657679a9ad) + * (GenCodeChecksum:504a7092bbd803d38b7f0fb91756bede) */ /** @@ -73,7 +73,7 @@ public function __construct() { * Whether to return the plural version of the title. */ public static function getEntityTitle($plural = FALSE) { - return $plural ? ts('State Provinces') : ts('State Province'); + return $plural ? ts('States/Provinces') : ts('State/Province'); } /** diff --git a/xml/schema/Core/StateProvince.xml b/xml/schema/Core/StateProvince.xml index 53f7aba31fa2..9d7d94e8864b 100644 --- a/xml/schema/Core/StateProvince.xml +++ b/xml/schema/Core/StateProvince.xml @@ -5,6 +5,8 @@ StateProvince civicrm_state_province 1.1 + State/Province + States/Provinces id State ID