Skip to content

Conversation

@mai-tran-03
Copy link

@mai-tran-03 mai-tran-03 commented Nov 4, 2024

Since the progress item background color is inconsistent in the light preview theme and is fine in other themes, I explicitly made DARK_THEME a custom theme so it would still have its color components while the background color is consistent in the light preview.

Fixes #2313

@mai-tran-03 mai-tran-03 changed the title Fixes to issue #2313 Fixes to issue #2313 Light (Preview) Theme Nov 4, 2024
@BeckerWdf
Copy link
Member

BeckerWdf commented Nov 4, 2024

getCustomThemeFlag was introduced with e60b130 which was a fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=506553

Copy link
Member

@opcoach opcoach left a comment

Choose a reason for hiding this comment

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

I rebased your contribution on master locally, launched it, and ran a 'clean all the projects' installed (I imported all the platform ui projects in my workspace). it is working well with or without a theme and whatever the theme chosen for the 'building' progress...

Please give a good name to the method used even if the method is private.

ITheme activeTheme = engine.getActiveTheme();
if (activeTheme != null) {
return !DEFAULT_THEME.equals(activeTheme.getId());
return DARK_THEME.equals(activeTheme.getId());
Copy link
Member

Choose a reason for hiding this comment

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

the getCustomThemeFlag() says in the javadoc that it is returning true if a theme is selected. But the method returns true if the current theme is the dark one. even if this is a private method, it could be renamed into 'isDarkThemeSelected'

@vogella
Copy link
Contributor

vogella commented Nov 27, 2024

@BeckerWdf is this PR included in or superseded by #2549?

@BeckerWdf
Copy link
Member

BeckerWdf commented Nov 28, 2024

@BeckerWdf is this PR included in or superseded by #2549?

no it's not.

@BeckerWdf
Copy link
Member

This PR does not touch the theme's css file at all. So it's not really related.

@mai-tran-03 mai-tran-03 force-pushed the mai_tran branch 2 times, most recently from 3b7d187 to 226fc15 Compare December 10, 2024 04:17
mai-tran-03 added a commit to mai-tran-03/eclipse.platform.ui that referenced this pull request Dec 10, 2024
@mai-tran-03
Copy link
Author

@opcoach Thank you for reviewing this pull request. I made the change you requested to rename the function. I accidentally closed this PR and I do not have permission to reopen it. Can you reopen it for me? I believe it is ready to be merged.

@opcoach
Copy link
Member

opcoach commented Mar 3, 2025

This PR is empty and has no more commit inside. I think you should create a new one. I can't reopen it as it is empty.

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.

Light (Preview) Theme: Issues in Progress View

4 participants