Skip to content

Fix view_menu.svg #2994

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.

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

Merged
merged 1 commit into from
May 19, 2025
Merged

Fix view_menu.svg #2994

merged 1 commit into from
May 19, 2025

Conversation

sratz
Copy link
Member

@sratz sratz commented May 19, 2025

view_menu.svg was the wrong svg file, not matching the .pngs.

Follow-up on #2925.

Supercedes and thus closes #2995.

@sratz sratz requested a review from BeckerWdf May 19, 2025 09:34
@sratz
Copy link
Member Author

sratz commented May 19, 2025

Was:
image

Should be:
image

@HeikoKlare
Copy link
Contributor

Just saw that we did the same thing at almost the same time :-)

However, note that the icon you propose is not correct. The existing SVG is not properly centered/sized. This is how it looks for me with your PR:
image

@sratz
Copy link
Member Author

sratz commented May 19, 2025

Oh wow, we have like a million different versions of the same svg lying around :D

@sratz sratz closed this May 19, 2025
@HeikoKlare
Copy link
Contributor

Oh wow, we have like a million different versions of the same svg lying around :D

Yeah, there were many SVGs for this, some were completely "wrong" (like the one you replaced), some just wrongly size. But actually I found none that completely fits to the existing PNGs. So the SVG I propose in #2995 is even a new one created out of the existing ones. Maybe I just missed the "correct" one lying around somewhere.

@BeckerWdf
Copy link
Contributor

Oh wow, we have like a million different versions of the same svg lying around :D

Ups. Sorry. Maybe I messed this up years ago without noticing.

@sratz sratz reopened this May 19, 2025
view_menu.svg was the wrong svg file, not matching the .pngs.

Follow-up on eclipse-platform#2925.

Uses the updated icons taken from
eclipse-platform/eclipse.platform.images#147
@sratz
Copy link
Member Author

sratz commented May 19, 2025

I updated all the icons with eclipse-platform/eclipse.platform.images#147, which is based on @HeikoKlare 's icon proposed in #2995.

Copy link
Contributor

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 41m 1s ⏱️ - 1m 32s
 7 925 tests ±0   7 697 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 862 runs  ±0  23 114 ✅ ±0  748 💤 ±0  0 ❌ ±0 

Results for commit 6dd5aa6. ± Comparison against base commit 710a26a.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Looks really great. Thank you, @sratz!

The icon now fits even better to the minimize/maximize icons next to them in the part stacks, as the stroke widths are better aligned. In addition, I tested on all fractional scales between 100% and 200% monitor zoom and it really looks good and much better than the previous PNG icon on all zoom levels.

Btw: the minimize/maximize icons are rendered programmatically. I quickly tried to do the same for the view_menu icon, but unfortunately GC-rendered circles do not look that good (even with anti-aliasing), which is why I cannot recommend trying out.

@sratz
Copy link
Member Author

sratz commented May 19, 2025

Thanks, @HeikoKlare for the tests and the initial version of the updated icon!

I suppose we are good to merge this for RC1?

@HeikoKlare
Copy link
Contributor

I suppose we are good to merge this for RC1?

Yes, +1 for doing so.

@sratz sratz merged commit c741652 into eclipse-platform:master May 19, 2025
18 checks passed
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