-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat(ui): enable windows mica effect #13476
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
Changes from 2 commits
f766e31
f01616f
a45f13f
c44d5e9
92ec3c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some test cases for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed unnecessary export
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some tests, not sure if they are necessary tho. I feel the main problem with current implementation is the way we get With that saying, worst case scenario, you simply lose Mica effect, nothing will break. Because: ...(windowsBackgroundMaterial ? { backgroundMaterial: windowsBackgroundMaterial } : {}),
backgroundColor: mainWindowBackgroundColor, |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,9 @@ import type { BrowserWindow } from 'electron' | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { isDev, isWin } from '../constant' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // see: https://www.electronjs.org/zh/docs/latest/api/base-window#winsetbackgroundmaterialmaterial-windows | ||||||||||||||||||||||
| const WINDOWS_11_22H2_BUILD = 22621 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| function isTilingWindowManager() { | ||||||||||||||||||||||
| if (process.platform === 'darwin') { | ||||||||||||||||||||||
| return false | ||||||||||||||||||||||
|
|
@@ -74,4 +77,19 @@ export const replaceDevtoolsFont = (browserWindow: BrowserWindow) => { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export const isWindowsMicaSupported = () => { | ||||||||||||||||||||||
| if (!isWin) { | ||||||||||||||||||||||
| return false | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const systemVersion = process.getSystemVersion() | ||||||||||||||||||||||
| const buildNumber = Number.parseInt(systemVersion.split('.')[2] ?? '', 10) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return Number.isFinite(buildNumber) && buildNumber >= WINDOWS_11_22H2_BUILD | ||||||||||||||||||||||
|
Comment on lines
+85
to
+88
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Only checking the build number while ignoring A more defensive approach would be to compare the full version tuple:
Suggested change
Not blocking — totally fine to keep as-is given Microsoft's current versioning. |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export const getWindowsBackgroundMaterial = () => { | ||||||||||||||||||||||
| return isWindowsMicaSupported() ? 'mica' : undefined | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export { isTilingWindowManager } | ||||||||||||||||||||||
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.
Should use consistent pattern like L91
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 will be 3 nested ternary operator if we choose to use it. Three conditions are
!isMac,!windowsBackgroundMaterial,nativeTheme.shouldUseDarkColors.Which is bad readability.
Or maybe you want to use an inline defined function?
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 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.
before this PR, it was
I didn't intend to change this. But sure, fixed now.