-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add checkbox column helper #89
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
Conversation
@@ -71,13 +75,15 @@ public AllFeaturesDemo() { | |||
|
|||
grid.setColumnReorderingAllowed(true); | |||
|
|||
grid.addColumn(Person::getLastName).setHeader("Last name").setHidingToggleCaption("Last name column"); | |||
grid.addColumn(Person::getLastName).setHeader("Last name") | |||
.setHidingToggleCaption("Last name column"); |
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.
Formatting changes in lines that are not modified
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.
fixed
grid.setSelectionMode(SelectionMode.MULTI); | ||
grid.setItems(TestData.initializeData()); |
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 moved, apparently unrelated to the implemented feature.
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.
fixed
@@ -178,12 +188,12 @@ public AllFeaturesDemo() { | |||
binder.forField(heightByRowsField).bind(this::getHeightByRows, this::setHeightByRows); | |||
updateHeightByRowsField(grid, heightByRowsField); | |||
heightByRowsField.setStepButtonsVisible(true); | |||
|
|||
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.
Formatting change
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.
fixed
@@ -0,0 +1,59 @@ | |||
:host([theme~='checkbox-top']) .vaadin-checkbox-container [part="checkbox"] { |
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.
Use some prefix such as fcgh-checkbox-top
in order to avoid collision with application-defined themes.
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.
fixed
*/ | ||
|
||
/* | ||
* This file incorporates work licensed under the Apache License, Version 2.0 from Vaadin Cookbook |
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.
Does it?
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.
definitely not, gonna remove it
*/ | ||
|
||
/* | ||
* This file incorporates work licensed under the Apache License, Version 2.0 from Vaadin Cookbook |
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.
Which recipe is this code based on?
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.
fixed
Kudos, SonarCloud Quality Gate passed!
|
Given that Vaadin's Grid doesn't provide a way to know when its dataprovider changes, if Select All checkbox is visible, and in order to keep it in sync with grid data, user must explicitly refresh the CheckboxColumn invoking refresh() method.
Close #69