replit / desktop

Replit Desktop App
114 stars 3 forks source link

[WS-31] windows integrated titlebar #102

Closed szymonkaliski closed 1 year ago

szymonkaliski commented 1 year ago

Why

Integrated titlebar on Windows like on macOS:

Screenshot 2023-08-15 at 15 15 12 Screenshot 2023-08-15 at 15 15 31

Needs https://github.com/replit/repl-it-web/pull/35147 to be merged.

What changed

Test plan

Check out the screenshots.

linear[bot] commented 1 year ago
WS-31 integrated titlebar/menubar UI for Windows, like on macOS

see what we can do, Windows looks ugly with the default title bar design (see screenshot) notes from Faris: [https://replit.slack.com/archives/C0509G0FJNL/p1683158097198949](https://replit.slack.com/archives/C0509G0FJNL/p1683158097198949) [Screenshot 2023-05-31 at 4.53.58 PM.png](https://uploads.linear.app/c5cab91b-5d7a-4b49-b62c-fe245c567e3b/52dcc342-5648-4d10-9a5c-9ea0a1277fb8/e5b7a77d-4b1b-4d3b-8029-34b4957aaf9b)

sergeichestakov commented 1 year ago

will check this in a bit with the web PR checked out - in the meantime looks like you have some conflicts here from the other PR

szymonkaliski commented 1 year ago

thanks; conflicts fixed :)

szymonkaliski commented 1 year ago

this largely looks good (except for the comment above) but can you make the background translucent? it looks like it's currently hardcoded to match the dark mode color so it looks off if you're using a light theme or anything other than our official dark theme

It's not hardcoded, it just defaults to the background/foreground from the dark theme, but then uses the colors recorded when we close the window:

https://github.com/replit/desktop/blob/e6a0243ba2f94bd02ebc966ae2454c3a87dcb21c/src/createWindow.ts#L171-L184

This obviously will break if we have a mismatch between stored/current colors, like if we change the theme without restarting the app.

One possible way forward would be to expose some version of https://www.electronjs.org/docs/latest/api/browser-window#winsettitlebaroverlayoptions-windows through the electron API, and allow the client to change the color when we change the theme. What do you think @sergeichestakov?

sergeichestakov commented 1 year ago

hmm it doesn't seem to restore for me in light mode (in other words, I still see the dark mode colors regardless). and yeah we're going to want to go with an approach that is resilient to switching themes

szymonkaliski commented 1 year ago

ok cool, I'll switch it to draft and will update soon

sergeichestakov commented 1 year ago

nvm looks like it does when I close and re-open a window (think I was just opening a window before without closing after switching themes)

szymonkaliski commented 1 year ago

nvm looks like it does when I close and re-open a window (think I was just opening a window before without closing after switching themes)

Yea that's what I was trying to say -- we only update when we close the window, so it's easy to get into a stale state, I think we should expose this through the API so we can switch colors dynamically.

szymonkaliski commented 1 year ago

@sergeichestakov I updated the code with comments, and added an event to notify on theme change, so we get this:

https://github.com/replit/desktop/assets/1040420/5af50f5f-f0fd-4e3f-9c75-7a2b26549f97

I tested on macOS and Windows, but would be good for you to test as well.

Since this depends on a new desktop app API event, you should test together with recent changes in https://github.com/replit/repl-it-web/pull/35147