-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Whitespace cleanup in template file #23277
Conversation
(Standard links)
|
<td class="form-item even-row{if $wizard.currentStepName == 'Preview'} labels{/if}"> | ||
{if $wizard.currentStepName == 'Preview'} | ||
{if $relatedContactDetails && $relatedContactDetails[$i] != ''}{$mapper[$i]} - {$relatedContactDetails[$i]} | ||
{if $relatedContactLocType && $relatedContactLocType[$i] != ''} - {$relatedContactLocType[$i]}{/if} |
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.
I find this harder to read than the original. Is this being done because the linebreaks affect the output?
e.g. this seems clearer to me:
{if $relatedContactDetails && $relatedContactDetails[$i] != ''}
{$mapper[$i]} - {$relatedContactDetails[$i]}
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.
@demeritcowboy hmm - I went with it because you can see all the ifs lined up & each one doesn't put much in - however, maybe that if is more important than the others so deserves the extra space - since the mapper is a bit 'more'
FYI - I'm hoping to move that work to the php layer
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.
Line 46 is also hard to read for the same reason - it looks like the minus sign is a subtraction operation.
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.
@demeritcowboy I fixed up the first one - but am less inclined to for the rest - because they all follow that same pattern of adding a - + one word & being able to skim the if's seems really useful
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.
Ok. It's hard to get consensus on visual preferences. Maybe we could do like wikipedia and make some bots that just keep editing each other's edits, so that all the readers are happy at least some of the time (grin).
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.
@demeritcowboy hopefully this code is on it's way out anyway - it was just doing my head in & once I re-laid it out I could see where the ifs started & finished
06da9e3
to
4e879d4
Compare
Overview
Whitespace cleanup in template file
Before
After
Technical Details
Just whitespace - although the diff is still a bit much due to also lines https://github.com/civicrm/civicrm-core/pull/23277/files?w=1
Comments