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

Refactored tag input renreder to fix rendering with empty options list #14319

Merged
merged 2 commits into from Feb 5, 2019

Conversation

alroniks
Copy link
Collaborator

What does it do?

It fixes the issue with displaying tag input component (TV) when options list set as empty.

Why is it needed?

It needed to avoid glitches that described in the issue.

Related issue(s)/PR(s)

#14177

@alroniks alroniks added this to the v2.7.1 milestone Jan 31, 2019
'value' => htmlspecialchars($itemValue,ENT_COMPAT,'UTF-8'),
'text' => htmlspecialchars($item,ENT_COMPAT,'UTF-8'),
'checked' => $checked,
list($label, $meaning) = $option;
Copy link
Collaborator

@Jako Jako Feb 1, 2019

Choose a reason for hiding this comment

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

Why not $value and $text? The variables are only used for placeholder values afterwards and could use their name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

$value already comes as the function parameter, I can not redefine it in the loop. About text yes, makes sense but label and meaning look more like a pair that text and meaning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I did not see that.

Meaning is somehow the wrong word. What about $tagText and $tagValue?

Copy link
Contributor

Choose a reason for hiding this comment

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

$inputOptionTextand $inputOptionValue?

@Jako Jako added bug The issue in the code or project, which should be addressed. area-core pr/review-needed Pull request requires review and testing. labels Feb 1, 2019
Mark-H
Mark-H previously approved these changes Feb 4, 2019
@alroniks
Copy link
Collaborator Author

alroniks commented Feb 4, 2019

Hi @Mark-H I've improved wording in variables a bit, please approve it again when you'll able to.

@Mark-H Mark-H added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Feb 5, 2019
@alroniks alroniks merged commit ce0312c into modxcms:2.x Feb 5, 2019
alroniks pushed a commit that referenced this pull request Feb 5, 2019
#14319]

* upstream/pr/14319:
  Improve wording in variables
  Refactored tag input renreder to fix rendering with empty options list
@alroniks alroniks deleted the bug-14177 branch February 5, 2019 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core bug The issue in the code or project, which should be addressed. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants