Skip to content
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

Add backgroundColor to saved state #74

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elsiehupp
Copy link

The purpose of this change is to mitigate the problem of Electron applications flashing white on startup. While the “correct” solution to the problem is using a 'ready-to-show’ event, adding this mitigation to a widely used (19,599 weekly downloads according to npm!) module should dramatically reduce the impact of this behavior in the wild.

As far as I can tell, the changes I made should work, but for reasons that I can’t really decode the JSON Truthy test is failing. (I am really not a JavaScript person, though, so I may have made some really obvious mistake.)

As a side note: I alphabetized the attributes in a couple of places for consistency with the original lists in index.d.ts, because I didn’t know where to place backgroundColor otherwise.

@elsiehupp
Copy link
Author

One thing I wasn't sure about was whether the default value for backgroundColor should be #fff (the default from Electron itself) or undefined. I think what it really comes down to is whether of not BrowserWindow() accepts undefined as the value for the parameter for backgroundColor, like so:

win = new BrowserWindow({
  'backgroundColor': undefined
});

If indeed BrowserWindow() accepts undefined as the value for the parameter for backgroundColor, then undefined would be the preferable default, as it would allow client code to distinguish between undefined (i.e. the default) and an explicitly set value of #fff.

@elsiehupp
Copy link
Author

What would actually be more helpful for my use case would be three attributes, rather than one:

backgroundColor: string
useSystemTheme: boolean
systemThemeBackgroundColorLight: string
systemThemeBackgroundColorDark: string

Then, for backgroundColor:

import { nativeTheme } from "electron";

get backgroundColor() {
    if (state.useSystemTheme) {
        if (nativeTheme.shouldUseDarkColors) {
            return state.systemThemeBackgroundColorDark;
        } else {
            return state.systemThemeBackgroundColorLight;
        }
    } else {
        return state.backgroundColor;
    }
}

The main issue here is that the three new variables would have to be set manually by the client code, but that shouldn't be insurmountable.

This is also getting messy, so I'll need to mull over it for a while longer.

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.

1 participant