FlowingCode / GridHelpers

Several grid recipes for Vaadin 23+ (and 22), ready to use. DOES NOT require extending Grid.
https://addonsv23.flowingcode.com/grid-helpers
Apache License 2.0
12 stars 1 forks source link

Add support to clear empty grid label #105

Closed flang closed 10 months ago

flang commented 11 months ago

Close #103

sonarcloud[bot] commented 11 months ago

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

javier-godoy commented 11 months ago

clearEmptyGridLabel keeps the component attached to the DOM but removes the association with the helper (it's a "component leak"), and now, I'm confused about why 8ae45df02cba1b2a294eee6d85df6f9b11b6186c was needed. The intent was that setEmptyGridLabel(null) would remove the label.

flang commented 11 months ago

Removing the empty grid label from its parent shouldn't be helper's responsibility IMHO. Helper should only make use of it.

javier-godoy commented 11 months ago

Removing the empty grid label from its parent shouldn't be helper's responsibility IMHO. Helper should only make use of it.

Good point. Helper does not add the label, hence removing it is not Helper's responsibility. However, there is a behavior change because calling setEmptyGridLabe twice will keep both components visible, but only the last label will synchronize with the provider..

Shouldn't it be implemented as follows? What do you think?

-    emptyLabel = component;
-    removeRegistration();
+   clearEmptyGridLabel();
+   emptyLabel = component;

I think there is an implicit contract that, whenever I call set, the old thing that has been set is now unset (whatever that means).

sonarcloud[bot] commented 10 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

flang commented 10 months ago

Let's just hide any existing empty grid label whenever set is call