Bug 553598 - [Tree] Fire a new EmptinessChanged event from Tree after first addition or last removal of a TreeItem
Summary: [Tree] Fire a new EmptinessChanged event from Tree after first addition or la...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.21   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.22 M1   Edit
Assignee: Eike Stepper CLA
QA Contact: Niraj Modi CLA
URL:
Whiteboard:
Keywords: noteworthy, plan
Depends on:
Blocks: 553353
  Show dependency tree
 
Reported: 2019-11-28 15:42 EST by Mickael Istria CLA
Modified: 2021-09-30 01:47 EDT (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2019-11-28 15:42:14 EST
From https://www.eclipse.org/lists/platform-dev/msg02007.html
"""
n the context of https://bugs.eclipse.org/bugs/show_bug.cgi?id=553353 , we'd like to be able to react to Tree emptiness or non-emptiness. Eike and I have tried different patches, but none of us could find an event to be informed of TreeItem addition to the root of the tree (deletion can be detected by Dispose, but there is no counter-part for addition).
[...]
If not, would an enhancement request about a new type of event SWT.ItemAdded and related listeners make sense?
"""
Comment 1 Eric Williams CLA 2019-11-28 15:57:52 EST
From the GTK POV is should be quite straight forward to implement an event that's fired when a Tree changes state from empty to non-empty.

Laskshmi/Niraj, what do you think about Mac/Windows?
Comment 2 Niraj Modi CLA 2019-11-29 04:00:57 EST
(In reply to Eric Williams from comment #1)
> From the GTK POV is should be quite straight forward to implement an event
> that's fired when a Tree changes state from empty to non-empty.
> 
> Laskshmi/Niraj, what do you think about Mac/Windows?

Hi Mickael,
Quick clarification:
'SWT.ItemAdded' event should be thrown only when a first time is added changing the state from empty to non-empty or an event on every new item added to a Tree/Table/Toolbar ?
Comment 3 Mickael Istria CLA 2019-11-29 04:04:56 EST
(In reply to Niraj Modi from comment #2)
> Quick clarification:
> 'SWT.ItemAdded' event should be thrown only when a first time is added
> changing the state from empty to non-empty or an event on every new item
> added to a Tree/Table/Toolbar ?

I don't have strong requirement here. Having the notification only when 1st item is added, or when any item is added would work for bug 553353. I just think event for any item addition seems more powerful and maybe easier to implement, but if you feel differently, I'm fine with it.
Just need to make sure that the case of addition of item, then removal, then addition sends a sequence of ItemAdded,Dispose,ItemAdded .
Comment 4 Andrey Loskutov CLA 2019-11-29 04:35:23 EST
Whoever plans to implement this API, please take care about mass additions - they should not cause event explosion.
Comment 5 Eric Williams CLA 2019-11-29 09:13:04 EST
(In reply to Andrey Loskutov from comment #4)
> Whoever plans to implement this API, please take care about mass additions -
> they should not cause event explosion.

Agreed, my thoughts is that this will be a single event fired when the Tree/Table/whatever changes from 0 items -> 1 item or more. Singular event, no more events fired for subsequent item additions.

The only case where we should re-fire the event is if all items are removed (item count goes to 0), and then an item is added.
Comment 6 Eike Stepper CLA 2019-11-29 11:48:17 EST
From an API point of view it would certainly be appealing to have the usual added/removed events, but I see the performance implications for mass updates. 

Good that our immidiate requirement is detection of "becoming empty" and "becoming non-empty". It would be great if a Tree could fire events for both these symmetric situations. That would relief us from registering a bunch of DisposeListeners, as well.
Comment 7 Eike Stepper CLA 2021-09-19 11:09:07 EDT
I'm going to submit a patch in a minute...

It would be great if this could be added soon in the 4.22 cycle because bug 553353 depends on it and surely needs discussions, too.
Comment 8 Eclipse Genie CLA 2021-09-19 11:11:11 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/185581
Comment 9 Eike Stepper CLA 2021-09-19 11:14:06 EDT
My patch adds the new SWT.EmptinessChanged event and fires it only once when the emptiness state of the tree changes, i.e., after first addition or last removal of a TreeItem. I've also added a test case that verifies this.
Comment 11 Eike Stepper CLA 2021-09-23 01:24:06 EDT
Thank you very much guys! I've tested again with today's I-build and it works very well. Can we resolve this bug now?
Comment 12 Andrey Loskutov CLA 2021-09-23 01:25:35 EDT
(In reply to Eike Stepper from comment #11)
> Thank you very much guys! I've tested again with today's I-build and it
> works very well. Can we resolve this bug now?

No, please provide N&N entry.
Comment 13 Eclipse Genie CLA 2021-09-23 01:48:45 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/185719
Comment 15 Eike Stepper CLA 2021-09-23 03:01:26 EDT
(In reply to Andrey Loskutov from comment #12)
> No, please provide N&N entry.

Where would that be?
Comment 16 Mickael Istria CLA 2021-09-23 03:05:53 EDT
(In reply to Eike Stepper from comment #15)
> Where would that be?

It's in https://git.eclipse.org/r/admin/repos/www.eclipse.org%2Feclipse%2Fnews , 4.22/platform_isv.html
Comment 17 Eclipse Genie CLA 2021-09-23 03:15:39 EDT
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/185721
Comment 18 Eclipse Genie CLA 2021-09-23 03:27:39 EDT
New Gerrit change created: https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/185724
Comment 20 Niraj Modi CLA 2021-09-23 04:11:18 EDT
(In reply to Eclipse Genie from comment #19)
> Gerrit change
> https://git.eclipse.org/r/c/www.eclipse.org/eclipse/news/+/185724 was merged
> to [master].
> Commit:
> http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/
> ?id=f280030ff062344ed584750aa7d9fe8b2ec40a52

Thanks Eike for the fix patch, added the SWT N&N entry.
Resolving now.

Something similar can be implemented for other SWT widgets:
For tracking raised bug 576210: for Table and bug 576210: for Toolbar widgets.
Comment 21 Niraj Modi CLA 2021-09-23 04:13:05 EDT
(In reply to Niraj Modi from comment #20)

Correction in bug numbers
bug 576210: for Table and bug 576211: for Toolbar widgets.
Comment 22 Mickael Istria CLA 2021-09-23 08:36:43 EDT
This cause API versioning issue, as reported by Ed on the mailing-list.
Comment 23 Niraj Modi CLA 2021-09-24 01:30:26 EDT
(In reply to Mickael Istria from comment #22)
> This cause API versioning issue, as reported by Ed on the mailing-list.

Taken care by commit in comment 14, API versioning issue is not seen in latest API tools report:
https://download.eclipse.org/eclipse/downloads/drops4/I20210923-1800/apitools/analysis/html/org.eclipse.swt/report.html
Comment 24 Niraj Modi CLA 2021-09-30 01:47:48 EDT
Verified in Eclipse IBuild: I20210929-1800 on Win10