-
Notifications
You must be signed in to change notification settings - Fork 38
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 custom window / native window controls #457
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a UI screenshot when menu location is set to Bottom
?
I have not handled that case here. If we want to support bottom menu on desktop, we will need another title bar - having the window drag from the bottom is not .. really an option from a UX perspective. I'll use the original window w/default Windows titlebar. I'll have to add something in to recreate the window (or require desktop restart). |
Ok then we probably want to disallow "Bottom" location of menu bar on electron app. |
2d631c0
to
1a6a8c7
Compare
@huchenlei Fixed the broken logic in this PR - it now just does what frontend tells it to, instead of forcing new default behaviour. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There doesn't seem to be a write to windowStyle
. Can you explain how is the store value initialized and updated?
There isn't one. That flag was for troubleshooting only. This PR was originally designed to simply replace the title bar, without any on/off switches. Allowing this requires destroying and recreating the app window; it would either require restarting the app, or a bit of re-architecting. Fixing this right now is not optimal. This PR will have to wait until other changes are in. |
Fix enum imported via meta file
Limitation: Cannot theme hover background of OS-controlled buttons - using a white background in dark mode appears to have no hover
Removes initial test height
It resets the height of the buttons, but the CSS is set explicitly to zero, breaking anything relying on it.
- Removes on-by-default behaviour, leaving no changes to default behaviour - Allows linked commits to happen async
8ec27a7
to
791d741
Compare
Previous comments are all resolved; API is complete, no changes to default behaviour. Impl. detail / UX can be handled entirely in frontend. |
Modern linux / win32 window
Example implementation:
windowStyle.mp4
┆Issue is synchronized with this Notion page by Unito