Skip to content
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

Enhanced control flow: Component level control for enabling and disabling events #396

Closed
vaadin-bot opened this issue Jan 17, 2008 · 5 comments
Labels
Stale Stale bot label

Comments

@vaadin-bot
Copy link
Collaborator

Originally by Jani Laakso


I feel that we need component wide disable events / enable events to our framework, just like other frameworks have.

It is needed for cases when you got listeners registered on your server-side code mainly taking care of receiving events of what client-side is doing (user clicks etc.) and you are doing large updates for your components' data containers on the server-side that have nothing to do with client-side events. In these cases I do not want to receive zillions of events.

Example:

  • you got a view V and Table T which's ValueChangeEvent listener is pointing to / implemented in V
  • ValueChangeEvent listener is required to get back event "table row clicked by user" => select row
  • table has 10 000 rows and 10 columns
  • now, on the server-side you refresh all item properties from the database
    => ISSUE: this results 100 000 calls to V.ValueChangeEventListener method which is very silly
  • Typical workaround: use flags that ignore event inside V.ValueChangeEventListener method (but still 100 000 calls)
  • Better workaround: use removeListener / addListeners before executing container data updates => no 100 000 calls, but this is quite a hack

I cannot override API in such way that I could myself add "ignoreEvents, allowEvents" methods per each component easily.

Perhaps adding this functionality would give users better control of what they are doing. I wonder if this kind of method is fundamentally too wrong? Either way, it's useful for many cases.

Typical pattern woudl be to:

private void refreshAllContainerRows() {
        
        // disable events because I know what I am doing
        (Table) bigTable.setEventsEnabled(false);
        
        try {
            // refresh all rows in the container
            (Container) bigContainer.refresh();
        } finally {
            // make sure events are finally enabled
            (Table) bigTable.setEventsEnabled(false);
        }
        
    }

Imported from https://dev.vaadin.com/ issue #1290

@vaadin-bot
Copy link
Collaborator Author

Originally by Jani Laakso


I think this is a bug in TK5, for some reason when I got a big table with one ValueChangeEventListener in my view, I get a hundred calls to my ValueChangeEventListener if I

  • remove all items in container
  • add 10 rows to table, each row has 10 columns

SUMMARY:
Issues seems to be: Modifying items or their properties in a container that is bind to a component should fire component's ValueChangeEventListener once or zero times depending on Table.getValue() state.

@vaadin-bot
Copy link
Collaborator Author

Originally by Jani Laakso


Better (but more time consuming) solution for "enhanced control flow" would be to create more (finer grained) events.

e.g. ValueChangeEvent is too abstract, we should have "ValueChangeEventOnServerSide" and "ValueChangeEventOnClientSide" styled events.

@vaadin-bot
Copy link
Collaborator Author

Originally by @jojule


Proposed KISS method of disabling events might be better.

One problem is that sometimes we do not know of all the listeners and should not disable events for them. One possible solution would be to collect all unique events in a queue while events are "disabled" and in the end send the queued events. This way all the listeners are guaranteed to be notified, but the performance problem is alleviated.

One possibility would be to implement both the above methods:
disableEvents(boolean queueUniqueEvents); enableEvents();

Also, enable disable API should probably count the number of disables in order to prevent intermediate sending in disable-disable-enable-disable-enable-enable case.

@stale
Copy link

stale bot commented Mar 21, 2018

Hello there!

It looks like this issue hasn't progressed lately. There are so many issues that we just can't deal them all within a reasonable timeframe.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Mar 21, 2018
@stale
Copy link

stale bot commented Sep 6, 2020

The issue was automatically closed due to inactivity. If you found some new details to it or started working on it, comment on the issue so that maintainers can re-open it.

@stale stale bot closed this as completed Sep 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Stale bot label
Projects
None yet
Development

No branches or pull requests

1 participant