Add data communicator API for disabling data provider interactions #9070
Conversation
mshabarov
added this to Iteration Reviews
in OLD Vaadin Flow ongoing work (Vaadin 10+)
via automation
There was a problem hiding this comment.
I think the implementation is done in a way that it will leak to the users (when using e.g. data view API) and causes errors that can confuse them. It should be basically about the component to be able to "delay" the first fetch to a certain point, but not cause errors for users.
| countCallback = null; | ||
| skipCountIncreaseUntilReset = false; | ||
| this.countCallback = null; | ||
| this.skipCountIncreaseUntilReset = false; |
There was a problem hiding this comment.
Unrelated unnecessary changes
| @@ -496,7 +537,7 @@ public void setItemCountEstimateIncrease(int itemCountEstimateIncrease) { | |||
| } | |||
| this.itemCountEstimateIncrease = itemCountEstimateIncrease; | |||
| this.countCallback = null; | |||
| definedSize = false; | |||
| this.definedSize = false; | |||
There was a problem hiding this comment.
Unrelated unnecessary change (yes the this. for countCallback is unnecessary too)
| } else if (index >= itemCount) { | ||
| throw new IndexOutOfBoundsException(String.format( | ||
| "Given index %d is outside of the accepted range '0 - %d'", | ||
| index, itemCount - 1)); | ||
| } | ||
| } | ||
|
|
||
| if (fetchDisabled) { |
There was a problem hiding this comment.
Why should this be blocked ??? Since this API is not directly exposed to users, like it should not, it feels weird to me that for some components I can do
Component listing = createListing();
listing.setItems(query -> getStuff(query));
listing.getItem(0);but not for all. So for Grid this would work and not for combobox (I guess) ??? That is confusing.
| * @return the size of data provider with current filter | ||
| */ | ||
| @SuppressWarnings({ "unchecked", "rawtypes" }) | ||
| protected int getDataProviderSize() { | ||
| if (fetchDisabled) { |
There was a problem hiding this comment.
Again, it would be weird for a data view API user that the same things work for another component, but not for another.
mshabarov
force-pushed
the
data-communicator-fetch-disable
branch
from
6a021a0
to
ec18d31
Compare
I was thinking whether it's better to use the data communicator as a listener for 'switch to normal mode', but I think it is a over-complication and basically limited to 1 component so far, so I simply left it as it is (CTOR arg + getter and setter). Curious users could of course try them randomly, but I have no elegant protection for that. |
|
https://www.jetbrains.com/help/idea/reformat-file-dialog.html
|
|
SonarQube analysis reported 2 issues Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
No description provided.