-
Notifications
You must be signed in to change notification settings - Fork 170
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
Revert theme changes #2206
Revert theme changes #2206
Conversation
Revert back to the 2024-06 state of the Eclipse Themes so that no changes are effective for the 2024-09 release. Once master opens up again we will bring the changes back and continue to improve them for the 2024-12 release.
Work on the improved light theme could not be finished for the 2024-09 release. To give user the possibility to already use it, this adds the improved theme as a preview theme. This also enables us to get early feedback from our users.
@merks, @mvm-sap, @jukzi, @Phillipus : Are you ok with this? |
Wasn't the vote to keep the new theme as default and add the old theme as additional one? One thing that will most likely not work with this approach is that the existing extensions, e.g. JDT syntax highlighting will not be picked up by the additional theme. AFAIR these extensions are based on the theme ID |
I've tested this change (extracting light theme change into a new "preview" theme) on Windows, and for me it works as expected.
I think the point is that only a preview for the light theme is provided, so theme extensions should be applied like for the default (light) theme. That would not work for the dark theme (which requires matching a specific theme ID), which is why there is no preview for the dark theme. Correct me if I am wrong, @BeckerWdf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks
@@ -51,6 +51,24 @@ | |||
label="%theme.win" | |||
os="win32"> | |||
</theme> | |||
<theme | |||
basestylesheeturi="css/e4_preview_gtk.css" | |||
id="org.eclipse.e4.ui.css.theme.e4_preview" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it would be worth finding it a new fresh id now that wouldn't change later. More precisely, the id shouldn't mention "preview". So later, when we want it to be default, we just change the default value of the preference and that's all.
People who have switched to this theme already wouldn't see an issue (the theme would still be present), and people who have never changed the theme should also see the update as the default value of the preference would change.
Maybe something like "flatLight" since this themes gets rid of many gradients, lines and constrasts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to copy the change to the "org.eclipse.e4.ui.css.theme.e4" and delete the "org.eclipse.e4.ui.css.theme.e4_preview" again. So preview is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you plan to remove the theme with id org.eclipse.e4.ui.css.theme.e4_preview
, then you may introduce issues to people who have enabled it and then update. The selected theme would then be missing.
To be safe, it's either changes in the current theme (but that couldn't be completed), or a new theme with a sustainable new id. Everything in between can be error-prone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this. If I have set the preview theme. Then start an eclipse with that workspace that does not have that theme it defaults to the "standard" light theme.
I am okay with what you folks decide is best. I think what @HeikoKlare has outlined is the safest strategy that avoids risk of a backlash and gives sufficient time to iron out the wrinkles. As we discussed at the community meeting, the poll was intended to be an opinion poll, not a binding vote. The technical experts should make a technical decision that is technically the best in their personal opinion, taking opinions of the community into account... |
That's why we did not provide a preview version of the dark theme. I think there are no theme extensions for the light theme. |
JDT already has an extension for light theme |
But it's a minor one. |
Why is "pr-head" red?
I want to merge this PR today. |
Rather looks like a glitch to me. The reference master build from yesterday shows the exact same number of fixed issues than this build shows as new issues, but I do not see how the commits added for that build should have resolved them: |
For me too... |
As written in #2199
these reverts the changes to the light and dark theme back to the 2024-06 release state.
In a second commit we provide the changes to the light theme as a new theme "Light (Preview)". This enables user to already test these changes and provide feedback.
Providing the changes to the dark theme as a new theme would be a bit more complicated. There are lots of downstream projects enhancing the dark theme (e.g. JDT defines lots of syntax colors for the dark theme). These downstream projects would also need to adapt to a new dark theme (providing their extension also for the new dark theme). This is simply not realistic to happen for the 2024-09 release.
Pls. review and test this PR thoroughly so that we can merge this as soon as possible for the 2024-09 release.