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
add null handling for item label properties #681
Conversation
Pull Request Test Coverage Report for Build 3369
💛 - Coveralls |
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.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @gatanaso)
src/vaadin-combo-box-dropdown-wrapper.html, line 262 at r1 (raw file):
if (label === undefined) { label = item ? item.toString() : ''; } else if (label === null) {
I feel that the correct code here should be like in the line above, set item.toString if item exists like it does with undefined.
4bf812f
to
67160d9
Compare
src/vaadin-combo-box-dropdown-wrapper.html, line 262 at r1 (raw file): Previously, manolo (Manuel Carrasco Moñino) wrote…
Thanks for the review. I have updated the PR. |
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.
Reviewable status: 0 of 2 files reviewed, all discussions resolved
src/vaadin-combo-box-dropdown-wrapper.html, line 262 at r1 (raw file):
Previously, gatanaso (Goran) wrote…
Thanks for the review. I have updated the PR.
Tip: if (label === undefined || label === null)
can be rewritten as if (label == null)
of if (label == undefined)
, but not a blocker.
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.
Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved
Rebase on current master, should help with passing the build. |
67160d9
to
e3a4e5b
Compare
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.
Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved
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.
Good Job! thanks for contributing
Reviewable status: complete! all files reviewed, all discussions resolved
This PR fixes #680 of the
vaadin-combo-box
failing to render when an item property used as a label path has anull
value.This change is