Skip to content

Add backgroundColor to saved state#74

Draft
elsiehupp wants to merge 1 commit intomawie81:masterfrom
elsiehupp:background-color
Draft

Add backgroundColor to saved state#74
elsiehupp wants to merge 1 commit intomawie81:masterfrom
elsiehupp:background-color

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.

Signed-off-by: Elsie Hupp <9206310+elsiehupp@users.noreply.github.com>
@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