-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
E-notice fix (smarty) #26975
E-notice fix (smarty) #26975
Conversation
Thank you for contributing to CiviCRM! ❤️ We will need to test and review the PR. 👷 Introduction for new contributors
Quick links for reviewers |
CRM/Custom/Page/Option.php
Outdated
} | ||
$this->assign('reusedNames', !empty($reusedNames) ? $reusedNames : NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eileenmcnaughton there's a style warning for the extra space before NULL
. But also, wouldn't this be simpler?
$this->assign('reusedNames', !empty($reusedNames) ? $reusedNames : NULL); | |
$this->assign('reusedNames', $reusedNames ?? NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colemanw so $reusedNames
would be an empty array at this point but I think it's kinda supposed tobe assigned as NULL to the template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should be string 'null'
...
Seriously it looks like it should be a string in the tpl but null works for the {if}
block.
d418829
to
7e03fec
Compare
Overview
E-notice fix (smarty)
Before
After
poof
Technical Details
Comments