[UI] Frameless window option (with theme support)

[0xCmdrKeen]

First PR, requesting feedback.

Summary

This adds a new setting "Use frameless window", which, if active during startup, will make Heroic create a frameless main window (i.e. no titlebar, window controls overlaid onto web content, see https://www.electronjs.org/docs/latest/tutorial/window-customization#create-frameless-windows).

Features

• Uses native overlay controls on macOS and Windows and custom overlay controls on Linux
• Makes the sidebar a draggable area on frameless windows (meaning windows can be moved around by dragging on the sidebar instead of the titlebar)
• The height and color of the overlaid window controls can be controlled by the theme, see Theming Support below
• Built-in themes have been updated to respect the the presence of overlay controls when running in frameless mode

Theming Support

The following variables can be set in a custom theme in order to change the appearance of the window controls:

• --titlebar-height: <number> will set the height of the window controls in pixels
• --titlebar-color: <hex color> will set the background color of the window controls (defaults to --body-background)
• --titlebar-symbol-color: <hex color> will set the foreground color of the window controls (defaults to --accent)

Also, the following variables are available to CSS authors for accommodating the presence of overlay controls:

• --overlay-controls-height: the actual height of the overlay controls, if present
• --overlay-controls-width: the actual width of the overlay controls, if present

Note that these variables are set regardless of whether frameless mode is active. If it IS enabled, an additional class frameless is added to the <App> element.

Screenshots

Windows 11, using Old School Heroic theme. Header automatically accommodates for missing space on the right.

Linux, using Old School Heroic theme and custom window controls.

MacOS, using Old School Heroic theme. No header adjustments necessary since window controls are on the left.

To Do

A couple of things I'm looking to improve on right off the bat:

• Remove duplication of functionality from getThemeCSS handler in main.ts#1578-1588
• Cleaner CSS parsing that doesn't rely on regular expressions
• If possible, allow references to other variables, since theme authors will likely want to use existing color definitions
• Add support for built-in themes
• Handle platform dependency (Linux doesn't support overlay controls)

Checklist

• Tested the feature and it's working on a current and clean install.
• Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
• Created / Updated Tests (If necessary)
• Created / Updated documentation (If necessary)

[ghost]

so good! this should be the norm for all apps possible

for linux controls i'd suggest a box icon for the maximize instead, since that's a better looking/more common design choice

[flavioislima]

Tested this one and for me it does not loads the windows controls on Linux:

[flavioislima]

Tried the appimage as well and same result. I am on KDE 5.27.

[0xCmdrKeen]

@flavioislima Indeed, can confirm, looks like the most recent commit accidentally broke this. I just pushed a fix for this, would you mind giving it another try?

[0xCmdrKeen]

We have the experimental features feature but I think this wouldn't work with that since this is already a toggleable option, we don't need to go with an experimental feature that would make it more complex. I think just adding a warning when somebody toggles the option on is enough, we do something like that for the Use dedicated GPU option when only 1 GPU is detected, you can use that for reference.

I imagine a confirmation message like "This feature is experimental, report any issue in GitHub", just to be extra safe.

Ok, can do. I was wondering whether it would make sense to put a dialog box here anyways to ask the user whether they want to relaunch the application right away after changing this setting. Could put a notice on that and remove it later.

[arielj]

Tested on linux with cinnamon and it works all good here 👍

I left 2 comments

I'm only a bit concerned that the window drag is now in the sidebar and I don't know how discoverable that is for users

maybe the cursor can change when hovering the draggable areas of the sidebar and not just when actually dragging?? that way when you are using the app, you hover, it changes the icon and you have a visual hint that you can drag

[arielj]

One I idea I want to implement is to inject some static information already in the preload script when we create the main window.

if you add something like this in src/backend/preload.ts:

contextBridge.exposeInMainWorld(
  'isSteamDeckGameMode',
  process.env.XDG_CURRENT_DESKTOP === 'gamescope'
)

then you can use window.isSteamDeckGameMode in the frontend anywhere without any async call

these system information will never change so it's safe to just inject like that and way simpler

I want to include more things there in the future but I think it's fine to start with that now that it's needed and can be refactored later

[0xCmdrKeen]

One I idea I want to implement is to inject some static information already in the preload script when we create the main window.

if you add something like this in src/backend/preload.ts:

contextBridge.exposeInMainWorld(
  'isSteamDeckGameMode',
  process.env.XDG_CURRENT_DESKTOP === 'gamescope'
)

then you can use window.isSteamDeckGameMode in the frontend anywhere without any async call

these system information will never change so it's safe to just inject like that and way simpler

Ah that's very interesting, I could have made use of that for the isFrameless flag as well instead of stringing that through the IPC layer and into the global context. I tried another approach using webContents.executeJavaScript(), but that ended up getting lost if the webContents were ever reloaded. I just tried this approach and it seems to stick.

Would it make sense to add another file to hold any other constants that might need to be made available on the frontend in the future? And should I use the same approach for isFrameless instead of the current one (since that can't change during runtime either)?

[arielj]

Would it make sense to add another file to hold any other constants that might need to be made available on the frontend in the future? And should I use the same approach for isFrameless instead of the current one (since that can't change during runtime either)?

That's what I want to do eventually so I think it's fine, anything that is constant that we need to expose to the frontend we could add that there and avoid a lot of boilerplate.

And I was thinking that even for non-constant values, we could always inject the initial state of the app with that to avoid the first IPC calls to get the initial states ready even before react loads.

[0xCmdrKeen]

Would it make sense to add another file to hold any other constants that might need to be made available on the frontend in the future? And should I use the same approach for isFrameless instead of the current one (since that can't change during runtime either)?

That's what I want to do eventually so I think it's fine, anything that is constant that we need to expose to the frontend we could add that there and avoid a lot of boilerplate.

And I was thinking that even for non-constant values, we could always inject the initial state of the app with that to avoid the first IPC calls to get the initial states ready even before react loads.

Yeah, that would be neat, the problem is what to name that file? There already is a constants.ts in same directory, and I'm not sure it's safe to just expose all of those to the frontend. If they go on window, we need to make sure not to overwrite anything important that's already there.

We could put another constants.ts in the api directory, but then they would all end up in the window.api namespace, which is already getting quite crowded (and might contain functions by the same name). Also wouldn't be super great to break pattern and load api/constants.ts directly into window in preload.ts.

Not quite sure what to do here. I'm quite tempted to just put that snippet into preload.ts directly (and perhaps add isFrameless there as well) until we have a better idea in the future.

[arielj]

Not quite sure what to do here. I'm quite tempted to just put that snippet into preload.ts directly (and perhaps add isFrameless there as well) until we have a better idea in the future.

I'll start with this, just add:

contextBridge.exposeInMainWorld(
  'isSteamDeckGameMode',
  process.env.XDG_CURRENT_DESKTOP === 'gamescope'
)

to preload.ts, we can refactor that into other files if we need to in the future

[flavioislima]

@0xCmdrKeen I believe we can merge this one before the next release, just need to fix the conflicts now.

[Etaash-mathamsetty]

there's an issue with electron 27 where the buttons just scroll up with the content, would be nice if that got fixed, I think it's the only issue

[0xCmdrKeen]

@Etaash-mathamsetty I can't seem to be able to reproduce this, however, after pulling the merge commit I am noticing that some screens are no longer using the full window height (see screenshot).

[arielj]

@Etaash-mathamsetty I can't seem to be able to reproduce this, however, after pulling the merge commit I am noticing that some screens are no longer using the full window height (see screenshot).

make sure to pull the latest latest changes from main, that should be fixed (that last merge is from before the fix I guess)

[0xCmdrKeen]

@Etaash-mathamsetty @arielj okay, this should take care of it.

[Kajot-dev]

With this feature enabled "Exit to System Tray" stops working on Linux (though I've not tested it on other platforms).
While closing the window with native controls Heroic stays minimized in the system tray (as expected).
While closing the frameless window with custom window controls Heroic terminates even though "Exit to System Tray" is enabled.

EDIT: But using Alt+F4 works correctly every time

[Etaash-mathamsetty]

With this feature enabled "Exit to System Tray" stops working on Linux (though I've not tested it on other platforms).
While closing the window with native controls Heroic stays minimized in the system tray (as expected).
While closing the frameless window with custom window controls Heroic terminates even though "Exit to System Tray" is enabled.

EDIT: But using Alt+F4 works correctly every time

Yep I can reproduce this, nice catch

[0xCmdrKeen]

While closing the frameless window with custom window controls Heroic terminates even though "Exit to System Tray" is enabled.

Hmm, I was using the same handleQuit helper that the "Quit" button in the sidebar uses. Looks like that actually bypasses the close handler that would minimize to the tray instead. I added another IPC function that calls the close handler instead, that should fix it.

[flavioislima]

I wonder, since this is an experimental feature, should it be under the Experimental Features Setting?

[flavioislima]

Another thing I found, not a huge deal since it is still experimental and optional. But it is not possible to resize the sidebar anymore. Also, the only way to move the window is by clicking don't the sidebar as well.

But that's ok, we can improve it later :)

[0xCmdrKeen]

I wonder, since this is an experimental feature, should it be under the Experimental Features Setting?

Yes, I also suggested this here, but @arielj was against it.

As far as not being able to resize the sidebar goes, that's a good catch. Might be a result of making it a draggable area, so I'm not entirely sure what if anything can be done about this, but honestly, I didn't even know the sidebar was resizable for the longest time, and I never really missed that feature, because the initial size seemed entirely appropriate to me.

[arielj]

it's just that adding it as a experimental feature is more to enable disable components, feels weird to enable the feature to show the toggle and then have to also enable the toggle?

[arielj]

also, the only reason I asked for that is because I had one issue that rendered heroic unusable, but I was not able to reproduce it in any way after that, so it was more to cover ourselves

[0xCmdrKeen]

@arielj I'm not trying to blame you, just pointing out a part of the discussion that @flavioislima might have missed.

Since it's just a simple toggle, it COULD go into the experimental features section, but I'm fine with either solution. Just cool to finally see this merged.