Ticket #1290 (new enhancement)

Opened 12 months ago

Last modified 8 months ago

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

Reported by: Jani Laakso Owned by: Joonas Lehtinen
Priority: major Milestone: IT Mill Sponsored Backlog
Component: Server-side framework Version: 5.2.0-rc
Keywords: Cc: marc.englund@…, matti.tahvonen@…
Known Issue description:
Hours estimate: 1 Deadline (dd.mm.yyyy):
Known Issue version (since): Known Issue title:
Hours done: Depends to:
Affects documentation: no
Known Issue workaround:
Affects release notes: yes Contract:

Description

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

Change History

Changed 12 months ago 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.

Changed 12 months ago 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.

Changed 12 months ago by Joonas Lehtinen

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.

Changed 8 months ago by Joonas Lehtinen

  • priority changed from undefined to major
  • version set to 5.2.0-rc
  • milestone set to IT Mill Sponsored Backlog
Note: See TracTickets for help on using tickets.