dev/core#5566 Fix clear options not showing for non-admin user #31705
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
|
The issue associated with the Pull Request can be viewed at https://lab.civicrm.org/dev/core/-/issues/5566 |
| {* Include the edit options list for admins *} | ||
| {if $formElement.html|strstr:"crm-option-edit-link"} | ||
| {$formElement.html|regex_replace:"@^.*(<a href=.*? class=.crm-option-edit-link.*?</a>)$@":"$1"} | ||
| {else} |
There was a problem hiding this comment.
This else is the actual change - all the rest is just adjusting the 2 templates to share the same code
'n' is not very meaningful. The 'sister' template uses profileFieldName - so let's align
eileenmcnaughton
force-pushed
the
dynamic
branch
from
df51620 to
b38144c
Compare
| {if $formElement.html|strstr:"crm-option-edit-link"} | ||
| {$formElement.html|regex_replace:"@^.*(<a href=.*? class=.crm-option-edit-link.*?</a>)$@":"$1"} | ||
| {else} | ||
| {$formElement.html} |
There was a problem hiding this comment.
This seems like it might duplicate things? I don't see this in the fields.tpl nor in dynamic.tpl previously
There was a problem hiding this comment.
or is this just the wrench?
There was a problem hiding this comment.
@seamuslee001 yeah so what was happening is that because the radio options were not being laid out as desired the tpl was iterating through & rendering the options html not the radio group html - and then using that to re-add some of the radio group html (but was dropping the clear when the edit was not in play)
Although this PR does mostly get us there I wound up concluding there is a better approach - ie update the renderer to add the classes - see #31706 (as of writing the PR name is not a good description of what it does)
…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()
Overview
dev/core#5566 Fix clear options not showing for non-admin user
Before
When a non admin user views a profile with a radio custom field in it with options_per_line set to 2 the clear link is missing. Depending on how the profile is accessed it may also be missing for non-admin users
After
Technical Details
This is confusing cos replication is hard - in part due to caching & also because viewing profiles in different ways accesses different tpls
Prior to this change the edit & clear never rendered at all in the former tpl and in the latter tpl the clear was not displayed unless the edit was ALSO displayed. This changes it so they both share a tpl snippet and the clear should always be present
Comments