From 34084de25b5646af3bdd2c96f2698f8a1e73017e Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 3 Jan 2025 14:31:14 +1300 Subject: [PATCH 1/3] Rename in-template variable for clarity 'n' is not very meaningful. The 'sister' template uses profileFieldName - so let's align --- templates/CRM/Profile/Form/Dynamic.tpl | 57 +++++++++++++------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/templates/CRM/Profile/Form/Dynamic.tpl b/templates/CRM/Profile/Form/Dynamic.tpl index 097066de54e6..bd01131424d0 100644 --- a/templates/CRM/Profile/Form/Dynamic.tpl +++ b/templates/CRM/Profile/Form/Dynamic.tpl @@ -61,7 +61,8 @@ {continue} {/if} {assign var="profileID" value=$field.group_id} - {assign var=n value=$field.name} + {assign var="profileFieldName" value=$field.name} + {if $field.groupTitle != $fieldset} {if $mode neq 8 && $mode neq 4}
@@ -75,7 +76,7 @@ {/if} {if $field.field_type eq "Formatting"} {$field.help_pre} - {elseif $n} + {elseif $profileFieldName} {if $field.groupTitle != $fieldset} {if $fieldset != $zeroField} {if $groupHelpPost} @@ -89,24 +90,24 @@ {/if}
{/if} - {if $field.help_pre && $action neq 4 && $form.$n.html} -
+ {if $field.help_pre && $action neq 4 && $form.$profileFieldName.html} +
{$field.help_pre}
{/if} {if array_key_exists('options_per_line', $field) && $field.options_per_line} -
-
{$form.$n.label}
+
+
{$form.$profileFieldName.label}
{assign var="count" value=1} {strip} {* sort by fails for option per line. Added a variable to iterate through the element array*} - {foreach name=outer key=key item=item from=$form.$n} + {foreach name=outer key=key item=item from=$form.$profileFieldName} {* There are both numeric and non-numeric keys mixed in here, where the non-numeric are metadata that aren't arrays with html members. *} {if is_array($item) && array_key_exists('html', $item)} - + {if $count == $field.options_per_line} @@ -123,21 +124,21 @@
{* end of main edit section div*} {else} -
+
- {$form.$n.label} + {$form.$profileFieldName.label}
- {if $n|substr:0:3 eq 'im-'} - {assign var="provider" value="$n-provider_id"} + {if $profileFieldName|substr:0:3 eq 'im-'} + {assign var="provider" value="$profileFieldName-provider_id"} {$form.$provider.html}  {/if} - {if $n eq 'email_greeting' or $n eq 'postal_greeting' or $n eq 'addressee'} + {if $profileFieldName eq 'email_greeting' or $profileFieldName eq 'postal_greeting' or $profileFieldName eq 'addressee'} {include file="CRM/Profile/Form/GreetingType.tpl"} - {elseif ( $n eq 'group' && $form.group ) || ( $n eq 'tag' && $form.tag )} - {include file="CRM/Contact/Form/Edit/TagsAndGroups.tpl" type=$n context="profile" tableLayout=1} - {elseif ( $form.$n.name eq 'image_URL' )} - {$form.$n.html} + {elseif ( $profileFieldName eq 'group' && $form.group ) || ( $profileFieldName eq 'tag' && $form.tag )} + {include file="CRM/Contact/Form/Edit/TagsAndGroups.tpl" type=$profileFieldName context="profile" tableLayout=1} + {elseif ( $form.$profileFieldName.name eq 'image_URL' )} + {$form.$profileFieldName.html} {if !empty($imageURL)}
@@ -145,19 +146,19 @@
{/if} - {elseif $n|substr:0:5 eq 'phone'} - {assign var="phone_ext_field" value=$n|replace:'phone':'phone_ext'} - {$form.$n.html} + {elseif $profileFieldName|substr:0:5 eq 'phone'} + {assign var="phone_ext_field" value=$profileFieldName|replace:'phone':'phone_ext'} + {$form.$profileFieldName.html} {if $form.$phone_ext_field.html}  {$form.$phone_ext_field.html} {/if} {else} {if $field.html_type neq 'File' || ($field.html_type eq 'File' && !$field.is_view)} - {$form.$n.html} + {$form.$profileFieldName.html} {/if} {if $field.html_type eq 'Autocomplete-Select'} {if $field.data_type eq 'ContactReference'} - {include file="CRM/Custom/Form/ContactReference.tpl" element_name = $n} + {include file="CRM/Custom/Form/ContactReference.tpl" element_name = $profileFieldName} {/if} {/if} {/if} @@ -165,17 +166,17 @@
- {if $form.$n.type eq 'file'} -
{$customFiles.$n.displayURL}
- {if !$fields.$n.is_view} -
{$customFiles.$n.deleteURL}
+ {if $form.$profileFieldName.type eq 'file'} +
{$customFiles.$profileFieldName.displayURL}
+ {if !$fields.$profileFieldName.is_view} +
{$customFiles.$profileFieldName.deleteURL}
{/if} {/if} {/if} {* Show explanatory text for field if not in 'view' mode *} - {if $field.help_post && $action neq 4 && $form.$n.html} -
+ {if $field.help_post && $action neq 4 && $form.$profileFieldName.html} +
{$field.help_post}
{/if} From b38144c1df368cda1fbf665a3adecd37ce9ebd26 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 3 Jan 2025 14:46:37 +1300 Subject: [PATCH 2/3] Fix clear radio not showing for non-admin user --- templates/CRM/Profile/Form/Dynamic.tpl | 31 +++---------------- .../CRM/UF/Form/FieldRadioOptionsPerLine.tpl | 20 ++++++++++++ templates/CRM/UF/Form/Fields.tpl | 18 +---------- 3 files changed, 25 insertions(+), 44 deletions(-) create mode 100644 templates/CRM/UF/Form/FieldRadioOptionsPerLine.tpl diff --git a/templates/CRM/Profile/Form/Dynamic.tpl b/templates/CRM/Profile/Form/Dynamic.tpl index bd01131424d0..c5b9a195be60 100644 --- a/templates/CRM/Profile/Form/Dynamic.tpl +++ b/templates/CRM/Profile/Form/Dynamic.tpl @@ -62,6 +62,8 @@ {/if} {assign var="profileID" value=$field.group_id} {assign var="profileFieldName" value=$field.name} + {assign var="rowIdentifier" value=$field.name} + {assign var="formElement" value=$form.$profileFieldName} {if $field.groupTitle != $fieldset} {if $mode neq 8 && $mode neq 4} @@ -96,34 +98,9 @@
{/if} {if array_key_exists('options_per_line', $field) && $field.options_per_line} -
-
{$form.$profileFieldName.label}
-
- {assign var="count" value=1} - {strip} -
{$form.$n.$key.html}{$form.$profileFieldName.$key.html}
- - {* sort by fails for option per line. Added a variable to iterate through the element array*} - {foreach name=outer key=key item=item from=$form.$profileFieldName} - {* There are both numeric and non-numeric keys mixed in here, where the non-numeric are metadata that aren't arrays with html members. *} - {if is_array($item) && array_key_exists('html', $item)} - - {if $count == $field.options_per_line} - - - {assign var="count" value=1} - {else} - {assign var="count" value=$count+1} - {/if} - {/if} - {/foreach} - -
{$form.$profileFieldName.$key.html}
- {/strip} -
-
+ {include file="CRM/UF/Form/FieldRadioOptionsPerLine.tpl"}
{* end of main edit section div*} - {else} + {else}
{$form.$profileFieldName.label} diff --git a/templates/CRM/UF/Form/FieldRadioOptionsPerLine.tpl b/templates/CRM/UF/Form/FieldRadioOptionsPerLine.tpl new file mode 100644 index 000000000000..52190ee8ce73 --- /dev/null +++ b/templates/CRM/UF/Form/FieldRadioOptionsPerLine.tpl @@ -0,0 +1,20 @@ +
+
{$formElement.label}
+
+
+ {foreach name=outer key=key item=item from=$formElement} + {if is_array($item) && array_key_exists('html', $item)} +
{$formElement.$key.html}
+ {/if} + {/foreach} +
+ {* Include the edit options list for admins *} + {if $formElement.html|strstr:"crm-option-edit-link"} + {$formElement.html|regex_replace:"@^.*()$@":"$1"} + {else} + {$formElement.html} + {/if} +
+
+
+
diff --git a/templates/CRM/UF/Form/Fields.tpl b/templates/CRM/UF/Form/Fields.tpl index 53e640f61e05..2fb7b8c72263 100644 --- a/templates/CRM/UF/Form/Fields.tpl +++ b/templates/CRM/UF/Form/Fields.tpl @@ -24,23 +24,7 @@
{/if} {if array_key_exists('options_per_line', $field) && $field.options_per_line != 0} - + {include file="CRM/UF/Form/FieldRadioOptionsPerLine.tpl"} {else}
From a0c54b83354ebce8211e301b9000d87c8c8ee14a Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 3 Jan 2025 16:51:55 +1300 Subject: [PATCH 3/3] Do not re-render the html in the error for radio fields with option values This does the same as https://github.com/civicrm/civicrm-packages/pull/416 but in Civi code rather than in packages. My take on it is that the original sin is that the quickform class for rendering radio options does not nest them in a div / allow us to add classes to that div and hence we are working around it by rendering the radio options individually in the template layer. If we fix that maybe we revert this - it is mostly a case of registering a class with more suitable markup in HTML_QuickForm::registerElementType() --- CRM/Core/BAO/CustomField.php | 3 +++ CRM/Core/Form.php | 4 +++- CRM/Core/Form/Renderer.php | 19 +++++++++++++++++-- templates/CRM/Form/errorLabel.tpl | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 templates/CRM/Form/errorLabel.tpl diff --git a/CRM/Core/BAO/CustomField.php b/CRM/Core/BAO/CustomField.php index 40bd26de4f05..973d1b97318a 100644 --- a/CRM/Core/BAO/CustomField.php +++ b/CRM/Core/BAO/CustomField.php @@ -775,6 +775,9 @@ public static function addQuickFormElement( if ($search || empty($useRequired)) { $fieldAttributes['allowClear'] = TRUE; } + if ($field->options_per_line) { + $fieldAttributes['options_per_line'] = $field->options_per_line; + } $qf->addRadio($elementName, $label, $options, $fieldAttributes, NULL, $useRequired); } break; diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index ee41275ecc3b..32d8475d84d3 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -1498,7 +1498,9 @@ public function &addRadio($name, $title, $values, $attributes = [], $separator = $options[] = $element; } $group = $this->addGroup($options, $name, $title, $separator); - + if (!empty($attributes['options_per_line'])) { + $group->setAttribute('options_per_line', $attributes['options_per_line']); + } $optionEditKey = 'data-option-edit-path'; if (!empty($attributes[$optionEditKey])) { $group->setAttribute($optionEditKey, $attributes[$optionEditKey]); diff --git a/CRM/Core/Form/Renderer.php b/CRM/Core/Form/Renderer.php index 01dc6bb0b866..2857ccf7a45e 100644 --- a/CRM/Core/Form/Renderer.php +++ b/CRM/Core/Form/Renderer.php @@ -84,6 +84,13 @@ public static function &singleton() { public function _elementToArray(&$element, $required, $error) { self::updateAttributes($element, $required, $error); + if ($element->getType() === 'group' && $element->getAttribute('options_per_line')) { + // Our standard template renders the erroneous html as part of it. + // This results in double renders in some cases - see https://lab.civicrm.org/dev/core/-/issues/5571 + // I suspect not rendering the html is probably often better but + // this adds it for our known problem case. + $this->setErrorTemplate('CRM/Form/errorClean.tpl'); + } $el = parent::_elementToArray($element, $required, $error); $el['textLabel'] = $element->_label ?? NULL; @@ -126,7 +133,13 @@ public function _elementToArray(&$element, $required, $error) { else { $typesToShowEditLink = ['select', 'group']; $hasEditPath = NULL !== $element->getAttribute('data-option-edit-path'); - + if ($element->getType() === 'group' && $element->getAttribute('options_per_line')) { + // Our standard template renders the erroneous html as part of it. + // This results in double renders in some cases - see https://lab.civicrm.org/dev/core/-/issues/5571 + // I suspect not rendering the html is probably often better but + // this adds it for our known problem case. + $this->setErrorTemplate('CRM/Form/errorClean.tpl'); + } if (in_array($element->getType(), $typesToShowEditLink) && $hasEditPath) { $this->addOptionsEditLink($el, $element); } @@ -135,7 +148,9 @@ public function _elementToArray(&$element, $required, $error) { $this->appendUnselectButton($el, $element); } } - + // We can't do a get to check but the only time we ever use a different template is within this + // function. + $this->setErrorTemplate('CRM/Form/error.tpl'); return $el; } diff --git a/templates/CRM/Form/errorLabel.tpl b/templates/CRM/Form/errorLabel.tpl new file mode 100644 index 000000000000..6295b3addb3f --- /dev/null +++ b/templates/CRM/Form/errorLabel.tpl @@ -0,0 +1,14 @@ +{* + +--------------------------------------------------------------------+ + | Copyright CiviCRM LLC. All rights reserved. | + | | + | This work is published under the GNU AGPLv3 license with some | + | permitted exceptions and without any warranty. For full license | + | and copyright information, see https://civicrm.org/licensing | + +--------------------------------------------------------------------+ +*} + +{if $error} + {$error} +{/if} +