-
-
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
[REF] minor refactor towards removing complexity. #15594
Conversation
This change actually makes the foreach & if following it redundant. I left them out for legibility but basically all the wrangling is on the 'other' contact. We do just one thing with the 'main' contact so the whole foreach & if is silly. Will follow up with removal of those
(Standard links)
|
$value[1] = NULL; | ||
} | ||
$rows["move_$field"]['main'] = self::getFieldValueAndLabel($field, $main)['label']; | ||
$rows["move_$field"]['other'] = self::getFieldValueAndLabel($field, $other)['label']; |
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.
The function getFieldValueAndLabel
does not appear to return an indexed array, so I don't think you can access ['label']
from its return values.
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.
Oh nevermind I see you changed that in this pr.
test this please |
Overview
Minor code legibility improvement.
Before
Less readable
After
More readable.
UI still works the same
Technical Details
This change actually makes the foreach & if following it redundant. I left them out for legibility
but basically all the wrangling is on the 'other' contact. We do just one thing with the 'main'
contact so the whole foreach & if is silly. This is the follow up to #15593 that incorporates removal of the if & foreach
Comments