Skip to content

Conversation

flang
Copy link
Contributor

@flang flang commented Aug 11, 2023

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

@@ -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");
Copy link
Member

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

Copy link
Contributor Author

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());
Copy link
Member

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.

Copy link
Contributor Author

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);

Copy link
Member

Choose a reason for hiding this comment

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

Formatting change

Copy link
Contributor Author

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"] {
Copy link
Member

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.

Copy link
Contributor Author

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
Copy link
Member

Choose a reason for hiding this comment

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

Does it?

Copy link
Contributor Author

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
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@paodb paodb requested a review from javier-godoy October 23, 2023 13:37
@javier-godoy javier-godoy merged commit 0818d26 into master Oct 24, 2023
@javier-godoy javier-godoy deleted the issue-69 branch October 24, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox column
2 participants