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

Set the selection to 1st CTabItem when it is created for the first time #1044

Merged
merged 1 commit into from
Mar 6, 2024
Merged

Set the selection to 1st CTabItem when it is created for the first time #1044

merged 1 commit into from
Mar 6, 2024

Conversation

deepika-u
Copy link
Contributor

Update the selectedIndex when the 1st CTabItem is created.

Fixes #46

Can one of you review please
@merks
@niraj-modi
@sravanlakkimsetti

@deepika-u deepika-u mentioned this pull request Feb 8, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 8, 2024 •
edited
Loading

Test Results

   299 files  ±0     299 suites  ±0   5m 46s ⏱️ -38s
 4 100 tests ±0   4 092 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 212 runs  ±0  12 137 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit 5ca254b. ± Comparison against base commit 596a08c.

♻️ This comment has been updated with latest results.

niraj-modi
niraj-modi reviewed Feb 8, 2024
View reviewed changes
Copy link
Member

@niraj-modi niraj-modi left a comment

Choose a reason for hiding this comment

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

Change looks good to me, please test and confirm below:

  1. various combination of Snippet76.java, changing order of CTabFolder and TabFolder
  2. Since this is a change in custom widget, test it on all 3 platforms(Win, Cocoa and Linux)
  3. Also Eclipse self hosted mode as well.

@deepika-u deepika-u changed the title Update the selectedIndex when the 1st CTabItem is created. Set the selection to 1st CTabItem when it is created for the first time Feb 20, 2024
@deepika-u
Copy link
Contributor Author

Now without the pr i see as below ->
image

With the pr 1044 ->
image

I have tested with TabFolder and CTabFolder order change via the "modified Snippet76", it all works fine now.

@niraj-modi
@merks
Can you please review now.

Can others also test this fix and let me know if that works for you on mac and linux please?
@Torbjorn-Svensson : can you let me know your opinion now please?

merks
merks requested changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

Removing the unnecessary this. qualifier I think would better match the style of the rest of the code.

...les/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java Outdated Show resolved Hide resolved
@deepika-u deepika-u force-pushed the Automatically_select_CTabItem0 branch from 9586db1 to bfc4175 Compare February 20, 2024 08:24
@deepika-u deepika-u force-pushed the Automatically_select_CTabItem0 branch 2 times, most recently from 0d87f61 to 731fe6c Compare February 27, 2024 06:50
@deepika-u deepika-u force-pushed the Automatically_select_CTabItem0 branch from 731fe6c to 5ca254b Compare February 27, 2024 06:50
lshanmug
lshanmug approved these changes Feb 27, 2024
View reviewed changes
Copy link
Member

@lshanmug lshanmug left a comment

Choose a reason for hiding this comment

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

LGTM

@lshanmug
Copy link
Member

@deepika-u Can you confirm if it was tested on Mac and Linux as well as it is common code for custom Control?

@deepika-u
Copy link
Contributor Author

Can anyone help me test this on Linux please?

@elsazac
Copy link
Member

elsazac commented Feb 27, 2024

@deepika-u The patch looks good on Mac. The first CTabItem is getting highlighted.

image

niraj-modi
niraj-modi approved these changes Mar 1, 2024
View reviewed changes
Copy link
Member

@niraj-modi niraj-modi left a comment

Choose a reason for hiding this comment

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

LGTM

@Torbjorn-Svensson
Copy link
Contributor

Now without the pr i see as below ->
image

With the pr 1044 ->
image

I have tested with TabFolder and CTabFolder order change via the "modified Snippet76", it all works fine now.

@niraj-modi
@merks
Can you please review now.

Can others also test this fix and let me know if that works for you on mac and linux please?
@Torbjorn-Svensson : can you let me know your opinion now please?

Sorry for the late reply.
I'm no longer working on our internal product and no longer have a working environment to verify this change, but based on the screenshots, it looks fine to me.

@niraj-modi niraj-modi merged commit 0532901 into eclipse-platform:master Mar 6, 2024
12 of 13 checks passed
@iloveeclipse iloveeclipse mentioned this pull request Mar 7, 2024
Closed
@iloveeclipse
Copy link
Member

This change causes regression, see eclipse-platform/eclipse.platform#1239
Please investigate.

@iloveeclipse iloveeclipse mentioned this pull request Mar 7, 2024
Closed
@iloveeclipse
Copy link
Member

Also please investigate another regression here: eclipse-platform/eclipse.platform.ui#1736

@jukzi
Copy link
Contributor

jukzi commented Mar 7, 2024

Please hurry with a fix or revert meanwhile because this regression also fails unrelated PRs in platform.ui.

@lshanmug
Copy link
Member

lshanmug commented Mar 7, 2024 •
edited
Loading

Let's revert this to unblock platform.ui while @deepika-u investigates the issues.

@lshanmug lshanmug mentioned this pull request Mar 7, 2024
Merged
@lshanmug
Copy link
Member

lshanmug commented Mar 7, 2024

This change has been reverted.

This was referenced Apr 4, 2024
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niraj-modi niraj-modi niraj-modi approved these changes

@lshanmug lshanmug lshanmug approved these changes

@merks merks merks approved these changes

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

Successfully merging this pull request may close these issues.

Automatically select first created CTabItem in a CTabFolder
8 participants
@deepika-u @lshanmug @elsazac @Torbjorn-Svensson @iloveeclipse @jukzi @merks @niraj-modi