Skip to content

Commit

Permalink
Do not re-render the html in the error for radio fields with option v…
Browse files Browse the repository at this point in the history
…alues

This does the same as civicrm/civicrm-packages#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()
  • Loading branch information
eileenmcnaughton committed Jan 3, 2025
1 parent b38144c commit a0c54b8
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CRM/Core/BAO/CustomField.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion CRM/Core/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
19 changes: 17 additions & 2 deletions CRM/Core/Form/Renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}

Expand Down
14 changes: 14 additions & 0 deletions templates/CRM/Form/errorLabel.tpl
Original file line number Diff line number Diff line change
@@ -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}
<span class="crm-error">{$error}</span>
{/if}

0 comments on commit a0c54b8

Please sign in to comment.