Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

Sorting Grid when backed by the LazyQueryContainer does nothing #71

Open
mvysny opened this issue Mar 9, 2015 · 7 comments
Open

Sorting Grid when backed by the LazyQueryContainer does nothing #71

mvysny opened this issue Mar 9, 2015 · 7 comments

Comments

@mvysny
Copy link

mvysny commented Mar 9, 2015

Hi, thank you for your wonderful extension. My problem is that when using the LQC with the new Vaadin 7.4 Grid, when clicking the Grid column to sort by a column, the LQC's sort() method only invalidates stuff but the data is not polled to the Grid and the Grid is not refreshed with the new data. I am not sure where the problem lies (whether it is in the Grid or in LQC) but I will keep looking. I can also attach a small sample project which illustrates this issue.
The intriguing part is that BeanContainer works correctly. The only difference to me is that BeanContainer fires the ItemSetChange event, while the LQC does not. However, Grid does not attach to the Container as a ItemSetChange listener AFAIK. I will try to explore this path though.

@mvysny
Copy link
Author

mvysny commented Mar 9, 2015

It seems that the ItemSetChange event actually is used, not by the Grid directly, but by the RpcDataProviderExtension class. There are actually two events, item set change event and property set change notifier, but I think that only the ItemSetChange is actually important. I will try to patch the LQC sources and I will let you know.

@mvysny
Copy link
Author

mvysny commented Mar 9, 2015

Patching LazyQueryContainer by adding notifyItemSetChanged(); to the sort() function helped.
From a quick glance on the filter-related methods I would say that the filters will suffer from identical issue.

@mvysny
Copy link
Author

mvysny commented Mar 9, 2015

I have verified that LazyQueryContainer.addContainerFilter(), LazyQueryContainer.removeContainerFilter() and LazyQueryContainer.removeAllContainerFilters() do not work properly with Grid unless notifyItemSetChanged(); is called.

@tlaukkan
Copy link
Owner

tlaukkan commented Mar 9, 2015

Hi

Nice work! It would be of great help if you could write a fix. Would you have time to make pull request?

Best regards,
Tommi

@mvysny
Copy link
Author

mvysny commented Mar 9, 2015

Sure, please let me clean up the code and study how to make a pull request :-) Also I will try the fix with the "old" Table, to check if nothing is broken by the fix.
Best regards,
Martin

@mvysny
Copy link
Author

mvysny commented Mar 12, 2015

Hi, created a pull request here: #72
It's my first pull request so I hope I did everything correctly ;)

tlaukkan added a commit that referenced this issue Mar 15, 2015
Fixes #71

Nice work, much appreciated. This then fulfills the requirements regarding filter modifications sending notification events automatically.
@tlaukkan
Copy link
Owner

tlaukkan commented Jun 17, 2016

A patch introduces behavior that refreshes container after each filter change which can run multiple unnecessary queries for example when multiple filters are removed. This behavior was introduced in a pull request and I missed the fact when accepting it. It is probably good time to think about adding functionality for mass removing filters in LazyQueryContainer API so that it disabled the filters at that point.

@tlaukkan tlaukkan reopened this Jun 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants