-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix focus issues on Windows after dialog interactions #16073
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
base: dev
Are you sure you want to change the base?
Conversation
app/electron/main.js
Outdated
}); | ||
ipcMain.on("siyuan-focus-fix", (event) => { | ||
const currentWindow = getWindowByContentId(event.sender.id); | ||
if (currentWindow && process.platform === 'win32') { |
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.
linux wayland 和 x11 也有这个问题,我印象中在思源的逻辑里面这个win32包含了win和linux,但是申请一下二次确认
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.
/**
* The `process.platform` property returns a string identifying the operating
* system platform for which the Node.js binary was compiled.
*
* Currently possible values are:
*
* * `'aix'`
* * `'darwin'`
* * `'freebsd'`
* * `'linux'`
* * `'openbsd'`
* * `'sunos'`
* * `'win32'`
*
* ```js
* import { platform } from 'process';
*
* console.log(`This platform is ${platform}`);
* ```
*
* The value `'android'` may also be returned if the Node.js is built on the
* Android operating system. However, Android support in Node.js [is experimental](https://github.com/nodejs/node/blob/HEAD/BUILDING.md#androidandroid-based-devices-eg-firefox-os).
* @since v0.1.16
*/
readonly platform: Platform;
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.
Linux 我测不了所以就不改了,你可以在 PR 合并之后本地测试一下再发一个 PR
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.
好的,那考虑之后处理。
一个小想法,就是一般GitHub上的hide功能只用来标记spam或者off-topic,你标成outdated以后我这边没办法把评论mark as resolve(或许是GitHub的bug)。
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.
这个不是手动选的,评论的代码如果已经不存在的话就会自动变成 Outdated
而且也并没有 resolve
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.
合并的话应该随时都能合并,不需要标记为解决
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.
不过GitHub的权限管理确实很混乱,我被封号期间甚至能看到员工超管的后台标签和按钮,就是按了没用。
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.
合并的话应该随时都能合并,不需要标记为解决
好的,那我就先不管咯。不过其他地方要把所有都标记为解决才能合并。可能这边不同
fix #16071 #12349
修复了使用
alert()
和confirm()
之后在 Windows 客户端中出现的焦点问题。修复之后焦点正常:video.webm