Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get help text title from field label if htxt title is not provided #26265

Conversation

larssandergreen
Copy link
Contributor

@larssandergreen larssandergreen commented May 19, 2023

Overview

This PR is for discussion, not sure the approach makes sense.
The aim is to avoid having to put the help text title for each help text in the .hlp files when the title is usually the field label. This would prevent the field label and help text title from getting out of sync if the field label changes and it makes it faster to add help texts. Question is if this is worthwhile or better just to keep copy-pasting the field names into the titles manually.

After

If the help text title is provided, no change.
If the help text title is not provided, the field label is used, providing the htxt id is id-field_name or id-field-name (both are used).

Bonus

Before if an extra.hlp file existed with a non-override title, it was appended without a space like TitleExtra. Now it is appended with a space like Title Extra, which is how the help text itself works.

@civibot
Copy link

civibot bot commented May 19, 2023

(Standard links)

@civibot civibot bot added the master label May 19, 2023
@larssandergreen larssandergreen changed the title WIP - Get help text title from field label if htxt title is empty WIP - Get help text title from field label if htxt title is not provided May 19, 2023
@larssandergreen larssandergreen changed the title WIP - Get help text title from field label if htxt title is not provided Get help text title from field label if htxt title is not provided May 19, 2023
@larssandergreen
Copy link
Contributor Author

jenkins, test this please

@colemanw
Copy link
Member

Technically the htxt id can be any string. By convention it begins with id- and I'm not even sure where that started, it's weird. So this feels a bit like string-fu-guesswork, but otoh it can be useful so I'm fine with it.
It would be good to add some kind of documentation or code comment somewhere about this.

@larssandergreen larssandergreen force-pushed the Add-title-to-form-elements-so-we-can-use-it-for-help-text-titles branch from 70bdbbb to 3af6469 Compare May 19, 2023 23:51
@larssandergreen
Copy link
Contributor Author

Definitely would want to add some documentation about this.

Could also get rid of the id string mess and simply look for an exact match to the field_name and just use that going forward. I don't see any reason we need the id in there.

@@ -67,6 +67,12 @@ function smarty_function_help($params, &$smarty) {
$name = trim(strip_tags($params['title']));
}

if (empty($name)) {
// sometimes the htxt id is field_name like the field, sometimes field-name instead so we fix it
$fieldID = str_replace('-', '_', str_replace('id-', '', $params['id']));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to suggest doing the id- part as either a preg_replace(/^id-/, or checking with substr that it's only removing ones at the start of the string, otherwise if it says something like paid-amount or covid-vaccine it won't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, have done.

@@ -67,6 +67,12 @@ function smarty_function_help($params, &$smarty) {
$name = trim(strip_tags($params['title']));
}

if (empty($name)) {
Copy link
Contributor

@demeritcowboy demeritcowboy Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have an extra.hlp file (usage described at #13488), the body part still works but for the title it ALWAYS overrides whether you put override=1 or not. I'm not sure how often titles are appended to rather than overwritten so probably not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easy enough to fix, have done. Also added a small bonus fix to the description above (or can break that out into a separate PR if preferred).

@larssandergreen larssandergreen force-pushed the Add-title-to-form-elements-so-we-can-use-it-for-help-text-titles branch from 3af6469 to 2b631b4 Compare June 6, 2023 15:22
@larssandergreen larssandergreen force-pushed the Add-title-to-form-elements-so-we-can-use-it-for-help-text-titles branch from 2b631b4 to 7abcf85 Compare June 6, 2023 15:28
@larssandergreen
Copy link
Contributor Author

Also documented the htxt override in docs.

@demeritcowboy
Copy link
Contributor

Thanks @larssandergreen looks pretty good.

Also documented the htxt override in docs.

It feels both right and wrong that the help feature needs its own help...

@demeritcowboy demeritcowboy merged commit 2c038c1 into civicrm:master Jun 6, 2023
@larssandergreen larssandergreen deleted the Add-title-to-form-elements-so-we-can-use-it-for-help-text-titles branch June 6, 2023 23:41
@larssandergreen
Copy link
Contributor Author

More help feature help in docs. Turns out the help feature was in fact undocumented, so to add the default title, I had to add it entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants