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.

Sign up for GitHub

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

Jump to bottom

CTabFolder should allow to listen on tab count changes #621

Merged
merged 1 commit into from Apr 11, 2023
Merged

CTabFolder should allow to listen on tab count changes #621

merged 1 commit into from Apr 11, 2023

Conversation

iloveeclipse
Copy link
Member

  • added CTabFolder2Listener.itemsCount(CTabFolderEvent) API
  • added CTabFolder2Listener.itemsCountAdapter(Consumer) API
  • updated CTabFolder to notify CTabFolder2Listener on creating/removing items
  • added unit test
  • bumped minor version segment for new API added

Fixes #620

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2023 •
edited

Test Results

     299 files  ±0       299 suites  ±0   5m 48s ⏱️ -3s
  4 049 tests +1    4 038 ✔️ +1    8 💤 ±0  3 ❌ ±0 
12 065 runs  +3  11 992 ✔️ +3  70 💤 ±0  3 ❌ ±0 

For more details on these failures, see this check.

Results for commit 55803c2. ± Comparison against base commit 435158d.

♻️ This comment has been updated with latest results.

This was referenced Apr 8, 2023
Merged
Merged
@iloveeclipse
Copy link
Member Author

@SyntevoAlex : any objections regarding the new API? I feel not OK to add API without anyone else looked at it.
The example IDE code that would use it is in https://github.com/eclipse-platform/eclipse.platform.ui/pull/710/files#diff-c065602fc1c20bba7ec8effcd0f47262b6cc7e9a32a9c9dc4b39bdfc993c5909R713

@SyntevoAlex
Copy link
Member

Sorry, I'm the wrong person to ask here.
My efforts are focused on our product, and we only use SWT, not Eclipse / eclipse.ui / etc.
So I'm not affected by API lifecycle issues an Eclipse use of these APIs.

@iloveeclipse
Copy link
Member Author

@SyntevoAlex : it is about SWT API review only.

SyntevoAlex
SyntevoAlex reviewed Apr 11, 2023
View reviewed changes
Copy link
Member

@SyntevoAlex SyntevoAlex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments are mostly nitpicks, so feel free to ignore.

...wt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java Outdated Show resolved Hide resolved
...les/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java Show resolved Hide resolved
...les/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java Outdated Show resolved Hide resolved
...clipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder2Listener.java Show resolved Hide resolved
@iloveeclipse
CTabFolder should allow to listen on tab count changes
55803c2 
- added CTabFolder2Listener.itemsCount(CTabFolderEvent) API
- added CTabFolder2Listener.itemsCountAdapter(Consumer<CTabFolderEvent>)
API
- updated CTabFolder to notify CTabFolder2Listener on creating/removing
items
- added unit test
- CTabFolderEvent should set "widget" field
- bumped minor version segment for new API added

Fixes eclipse-platform#620
@iloveeclipse
Copy link
Member Author

Thanks Alex for review.

@iloveeclipse iloveeclipse merged commit 437e501 into eclipse-platform:master Apr 11, 2023
10 of 13 checks passed
@iloveeclipse iloveeclipse deleted the issue_620 branch April 11, 2023 16:05
laeubi
laeubi reviewed Apr 16, 2023
View reviewed changes
...les/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java
@@ -744,6 +744,14 @@ Image createButtonImage(Display display, int button) {
image = new Image(display, new AutoScaleImageDataProvider(display, imageData, DPIUtil.getDeviceZoom()));
return image;
}

private void notifyItemCountChange() {
CTabFolderEvent e = new CTabFolderEvent(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could init the event lazy in the loop, so if there are no listeners you don't create a uselsess object that is instantly garbage collected.
Alternatively, if folderListeners.isEmpty() one might early exit this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laeubi laeubi laeubi left review comments

@SyntevoAlex SyntevoAlex SyntevoAlex left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CTabFolder should allow to listen on tab count changes
3 participants
@iloveeclipse @SyntevoAlex @laeubi