Skip to content

Conversation

andrew-tram
Copy link

@andrew-tram andrew-tram commented Aug 13, 2025

Proposed solution for #3069

Currently, the code decides whether to show the onboarding image using a fixed threshold (ONBOARDING_SHOW_IMAGE_HEIGHT_THRESHOLD, 450px content height), regardless of the actual image size.

My idea is to make this more adaptive by retrieve the actual image height from the label.

If the image is small (<= 250px), allow it to be shown with a lower overall height requirement (e.g., 314px total window height).
If the image is large (> 250px), keep using the existing higher threshold.

Also, from the extension point descrption:

The image is shown in the editor area in case no editor is open.
The image shall be grey and not colored and shall have a size of 250 x 250 px.
Plus a second image for high resolution with a size of 500 x 500 px and a name like [image_name]@2x.[image_type].

I dont really see it take into account the @2x size for an image, so when I manually use a 500x500 image, it gets cut off anyway because of the 450 threshold. But maybe I dont know how to trigger the @2x resolution in Eclipse properly.

@andrew-tram
Copy link
Author

Hey all, looking for feedback and review on this PR.

@HeikoKlare
Copy link
Contributor

Sound like a reasonable intermediate solution until we have:

@marcushoepfner this is probably most interesting for you and particularly interesting to have your opinion

@BeckerWdf
Copy link
Contributor

@marcushoepfner currently is on vacation. So he will not be able to react here in a timely manner.

@HeikoKlare
Copy link
Contributor

Thanks for the info! I guess it's not a problem as this cannot go into the upcoming release anymore anyway, so we basically have time until November to finalize it for the December release.

@BeckerWdf
Copy link
Contributor

Thanks for the info! I guess it's not a problem as this cannot go into the upcoming release anymore anyway, so we basically have time until November to finalize it for the December release.

Yes I agree that this is not a problem. Just wanted to set expectations.

@andrew-tram
Copy link
Author

Hey all! It sounds like this has potential for post 2025-09?

Copy link
Contributor

Test Results

 1 179 files   -  1 599   1 179 suites   - 1 599   20m 31s ⏱️ - 1h 25m 39s
 3 682 tests  -  4 250   3 663 ✅  -  4 041  19 💤  - 209  0 ❌ ±0 
10 599 runs   - 12 750  10 529 ✅  - 12 074  70 💤  - 676  0 ❌ ±0 

Results for commit 6841a0e. ± Comparison against base commit 5922d0c.

This pull request removes 4250 tests.
RcpTestSuite ActionBarConfigurerTest ‑ testDefaults
RcpTestSuite IWorkbenchPageTest ‑ test70080
RcpTestSuite PlatformUITest ‑ testCreateAndRunWorkbench
RcpTestSuite PlatformUITest ‑ testCreateAndRunWorkbenchWithExceptionOnStartup
RcpTestSuite PlatformUITest ‑ testCreateDisplay
RcpTestSuite PlatformUITest ‑ testEarlyGetWorkbench
RcpTestSuite WorkbenchAdvisorTest ‑ testCloseFromPostStartup
RcpTestSuite WorkbenchAdvisorTest ‑ testEarlyGetWorkbench
RcpTestSuite WorkbenchAdvisorTest ‑ testEmptyProgressRegion
RcpTestSuite WorkbenchAdvisorTest ‑ testEventLoopCrash
…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants