Skip to content

Provide Explicit "isDarkTheme" Attribute #2808

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: org.eclipse.e4.ui.css.swt.theme;singleton:=true
Bundle-Version: 0.14.500.qualifier
Bundle-Version: 0.14.600.qualifier
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@
</documentation>
</annotation>
</attribute>
<attribute name="isDarkTheme" type="boolean">
<annotation>
<documentation>
Set this to true if the theme uses dark background.
</documentation>
</annotation>
</attribute>
</complexType>
</element>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class Theme implements ITheme {
private String id;
private String label;
private String osVersion;
private String isDark;

public Theme(String id, String label) {
this.id = id;
Expand All @@ -43,10 +44,25 @@ public String getOsVersion() {
return this.osVersion;
}

@Override
public boolean isDark() {
if (isDark == null) {
// fallback for themes that don't yet set the "isDark" attribute
// will be removed in a later release
return id.contains("dark");
} else {
return Boolean.parseBoolean(isDark);
}
}

public void setIsDark(String isDark) {
this.isDark = isDark;
}

@Override
public String toString() {
return "Theme [id=" + id + ", label='" + label + "', osVersion="
+ osVersion + "]";
+ osVersion + " isDark=" + isDark + "]";
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public class ThemeEngine implements IThemeEngine {
private HashMap<String, List<IResourceLocator>> sourceLocators = new HashMap<>();

private static final String THEMEID_KEY = "themeid";
private static final String THEME_IS_DARK_KEY = "themeIsDark";

public static final String THEME_PLUGIN_ID = "org.eclipse.e4.ui.css.swt.theme";

Expand Down Expand Up @@ -121,6 +122,7 @@ public ThemeEngine(Display display) {
String id = ce.getAttribute("id");
String os = ce.getAttribute("os");
String version = ce.getAttribute("os_version");
String isDarkTheme = ce.getAttribute("isDarkTheme");

/*
* Code to support e4 dark theme on Mac 10.13 and older. For e4 dark theme on
Expand Down Expand Up @@ -154,7 +156,8 @@ public ThemeEngine(Display display) {
basestylesheeturi = "platform:/plugin/" + ce.getContributor().getName() + "/"
+ basestylesheeturi;
}
registerTheme(themeId, label, basestylesheeturi, version);

registerTheme(themeId, label, basestylesheeturi, version, isDarkTheme);

//check for modified files
if (modifiedFiles != null) {
Expand Down Expand Up @@ -250,11 +253,11 @@ private boolean isOsVersionMatch(String osVersionList) {
@Override
public synchronized ITheme registerTheme(String id, String label, String basestylesheetURI)
throws IllegalArgumentException {
return registerTheme(id, label, basestylesheetURI, "");
return registerTheme(id, label, basestylesheetURI, "", "");
}

public synchronized ITheme registerTheme(String id, String label,
String basestylesheetURI, String osVersion) throws IllegalArgumentException {
String basestylesheetURI, String osVersion, String isDark) throws IllegalArgumentException {
for (Theme t : themes) {
if (t.getId().equals(id)) {
throw new IllegalArgumentException("A theme with the id '" + id
Expand All @@ -265,6 +268,9 @@ public synchronized ITheme registerTheme(String id, String label,
if (osVersion != "") {
theme.setOsVersion(osVersion);
}
if (isDark != "") {
theme.setIsDark(isDark);
}
themes.add(theme);
registerStyle(id, basestylesheetURI, false);
return theme;
Expand Down Expand Up @@ -496,6 +502,7 @@ public void setTheme(ITheme theme, boolean restore, boolean force) {
EclipsePreferencesHelper.setCurrentThemeId(theme.getId());

pref.put(THEMEID_KEY, theme.getId());
pref.putBoolean(THEME_IS_DARK_KEY, theme.isDark());
try {
pref.flush();
} catch (BackingStoreException e) {
Expand Down Expand Up @@ -560,6 +567,10 @@ private String getPreferenceThemeId() {
return getPreferences().get(THEMEID_KEY, null);
}

private boolean getPreferenceThemeIsDark(boolean fallback) {
return getPreferences().getBoolean(THEME_IS_DARK_KEY, fallback);
}

private IEclipsePreferences getPreferences() {
return InstanceScope.INSTANCE.getNode(FrameworkUtil.getBundle(ThemeEngine.class).getSymbolicName());
}
Expand Down Expand Up @@ -593,12 +604,24 @@ public void restore(String alternateTheme) {
for (ITheme t : getThemes()) {
if (prefThemeId.equals(t.getId())) {
setTheme(t, false);

// also update "isDark" attribute in the pref-store (might be not set there)
// will be removed in a later release
IEclipsePreferences preferences = getPreferences();
if (preferences.get(THEME_IS_DARK_KEY, null) == null) {
preferences.putBoolean(THEME_IS_DARK_KEY, t.isDark());
try {
preferences.flush();
} catch (BackingStoreException e) {
ThemeEngineManager.logError(e.getMessage(), e);
}
}
return;
}
}
}

boolean hasDarkTheme = getThemes().stream().anyMatch(t -> t.getId().startsWith(E4_DARK_THEME_ID));
boolean hasDarkTheme = getThemes().stream().anyMatch(t -> t.isDark());
boolean overrideWithDarkTheme = false;
if (hasDarkTheme) {
if (prefThemeId != null) {
Expand All @@ -607,7 +630,7 @@ public void restore(String alternateTheme) {
* this case want to fall back to respect whether that previous choice was dark
* or not. https://github.com/eclipse-platform/eclipse.platform.ui/issues/2776
*/
overrideWithDarkTheme = prefThemeId.contains("dark");
overrideWithDarkTheme = getPreferenceThemeIsDark(prefThemeId.contains("dark"));
} else {
/*
* No previous theme selection in preferences. In this case check if the system
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ public interface ITheme {
* @return the label
*/
String getLabel();

/**
*
* @return does the theme use a dark background color
*/
boolean isDark();
}
2 changes: 1 addition & 1 deletion bundles/org.eclipse.e4.ui.swt.gtk/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %fragmentName
Bundle-SymbolicName: org.eclipse.e4.ui.swt.gtk;singleton:=true
Bundle-Version: 1.2.200.qualifier
Bundle-Version: 1.2.300.qualifier
Fragment-Host: org.eclipse.e4.ui.css.swt.theme;bundle-version="0.10.0"
Bundle-RequiredExecutionEnvironment: JavaSE-17
Bundle-Localization: fragment-gtk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,11 @@ public void intialize() {
return;
}
ITheme theme = (ITheme) event.getProperty("theme");
final boolean isDark = theme.getId().contains("dark"); //$NON-NLS-1$
Display display = (Display) event.getProperty(IThemeEngine.Events.DEVICE);

// not using UISynchronize as this is specific to SWT/GTK
// scenarios
display.asyncExec(() -> OS.setDarkThemePreferred(isDark));
display.asyncExec(() -> OS.setDarkThemePreferred(theme.isDark()));
};
// using the IEventBroker explicitly because the @EventTopic annotation
// is unpredictable with processors within the debugger
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.e4.ui.swt.win32/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %fragmentName
Bundle-SymbolicName: org.eclipse.e4.ui.swt.win32;singleton:=true
Bundle-Version: 1.2.300.qualifier
Bundle-Version: 1.2.400.qualifier
Fragment-Host: org.eclipse.e4.ui.css.swt.theme;bundle-version="0.10.0"
Bundle-RequiredExecutionEnvironment: JavaSE-17
Bundle-Localization: fragment-win32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ public void intialize() {
return;
}
ITheme theme = (ITheme) event.getProperty("theme");
boolean isDark = theme.getId().contains("dark"); //$NON-NLS-1$
OS.setTheme (isDark);
OS.setTheme (theme.isDark());
};
// using the IEventBroker explicitly because the @EventTopic annotation
// is unpredictable with processors within the debugger
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Eclipse-PlatformFilter: (osgi.ws=cocoa)
Bundle-SymbolicName: org.eclipse.e4.ui.workbench.renderers.swt.cocoa;singleton:=true
Bundle-Version: 0.14.400.qualifier
Bundle-Version: 0.14.500.qualifier
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Fragment-Host: org.eclipse.e4.ui.workbench.renderers.swt;bundle-version="[0.10.0,1.0.0)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,23 @@ public void intialize() {
return;
}
ITheme theme = (ITheme) event.getProperty("theme"); //$NON-NLS-1$
final boolean isDark = theme.getId().contains("dark"); //$NON-NLS-1$
Display display = (Display) event.getProperty(IThemeEngine.Events.DEVICE);

// not using UISynchronize as this is specific to SWT/Mac
// scenarios
display.asyncExec(() -> OS.setTheme(isDark));
display.asyncExec(() -> OS.setTheme(theme.isDark()));
};
// using the IEventBroker explicitly because the @EventTopic annotation
// is unpredictable with processors within the debugger
eventBroker.subscribe(IThemeEngine.Events.THEME_CHANGED, eventHandler);
}

/**
* Unsubscribes the {@code eventHandler} from the {@code eventBroker} to cleanup the resources
* Unsubscribes the {@code eventHandler} from the {@code eventBroker} to cleanup
* the resources
*
* Assumption : Both {@code eventHandler} and {@code eventBroker} are initialized and non null
* Assumption : Both {@code eventHandler} and {@code eventBroker} are
* initialized and non null
*/
@PreDestroy
public void cleanUp() {
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Plugin.name
Bundle-SymbolicName: org.eclipse.ui.themes;singleton:=true
Bundle-Version: 1.2.2700.qualifier
Bundle-Version: 1.2.2800.qualifier
Bundle-Vendor: %Plugin.providerName
Bundle-Localization: plugin
Require-Bundle: org.eclipse.e4.ui.css.swt.theme
Expand Down
38 changes: 21 additions & 17 deletions bundles/org.eclipse.ui.themes/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,33 @@
label="%theme.classic">
</theme>
<theme
basestylesheeturi="css/e4-dark_linux.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
label="%theme.dark"
os="linux">
basestylesheeturi="css/e4-dark_linux.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
isDarkTheme="true"
label="%theme.dark"
os="linux">
</theme>
<theme
basestylesheeturi="css/e4-dark_win.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
label="%theme.dark"
os="win32">
basestylesheeturi="css/e4-dark_win.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
isDarkTheme="true"
label="%theme.dark"
os="win32">
</theme>
<theme
basestylesheeturi="css/e4-dark_mac1013.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
label="%theme.dark"
os="macosx"
os_version="10.11,10.12,10.13">
basestylesheeturi="css/e4-dark_mac1013.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
isDarkTheme="true"
label="%theme.dark"
os="macosx"
os_version="10.11,10.12,10.13">
</theme>
<theme
basestylesheeturi="css/e4-dark_mac.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
label="%theme.dark"
os="macosx">
basestylesheeturi="css/e4-dark_mac.css"
id="org.eclipse.e4.ui.css.theme.e4_dark"
isDarkTheme="true"
label="%theme.dark"
os="macosx">
</theme>
<theme
basestylesheeturi="css/e4_default_gtk.css"
Expand Down
2 changes: 1 addition & 1 deletion tests/org.eclipse.e4.ui.tests.css.swt/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Bundle-Name
Bundle-SymbolicName: org.eclipse.e4.ui.tests.css.swt; singleton:=true
Bundle-Version: 0.12.800.qualifier
Bundle-Version: 0.12.900.qualifier
Require-Bundle: org.eclipse.e4.ui.css.core,
org.eclipse.e4.ui.css.swt,
org.eclipse.e4.ui.css.swt.theme;bundle-version="0.9.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public void setUp() {
@Override
@AfterEach
public void tearDown() {
themeListenerRegistration.unregister();
if (themeListenerRegistration != null) {
themeListenerRegistration.unregister();
}
super.tearDown();
}

Expand Down Expand Up @@ -94,4 +96,32 @@ private IThemeEngine getThemeEngine(Display display) {
return manager.getEngineForDisplay(display);
}

@Test
void settingIsDarkToTrueShoulReportThemeIsDark() {
Theme theme = new Theme("IdDoesntCare", "DescriptionDoesntCare");
theme.setIsDark("true");
assertTrue(theme.isDark(), "Theme should report to be dark");
}

@Test
void settingIsDarkToFalseShoulReportThemeIsNotDark() {
Theme theme = new Theme("IdDoesntCare", "DescriptionDoesntCare");
theme.setIsDark("false");
assertFalse(theme.isDark(), "Theme should report to be NOT dark");
}

@Test
void settingIsDarkToNullShoulReportThemeIsNotDark() {
Theme theme = new Theme("IdDoesntCare", "DescriptionDoesntCare");
theme.setIsDark(null);
assertFalse(theme.isDark(), "Theme should report to be NOT dark");
}

@Test
void settingIsDarkToInvalidValueShoulReportThemeIsNotDark() {
Theme theme = new Theme("IdDoesntCare", "DescriptionDoesntCare");
theme.setIsDark("invalid");
assertFalse(theme.isDark(), "Theme should report to be NOT dark");
}

}
Loading