-
-
Notifications
You must be signed in to change notification settings - Fork 369
fix and feat #4164
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: master
Are you sure you want to change the base?
fix and feat #4164
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -289,6 +289,8 @@ class Windows { | |
// note: for some reason, it behaves differently if you minimize the tab group after moving it to another space | ||
if let cgWindowId = window.cgWindowId { | ||
let spaceIds = cgWindowId.spaces() | ||
window.spaceIds = spaceIds | ||
debugPrint("44|",window.cgWindowId!,spaceIds,window.spaceId,window.title!,Spaces.currentSpaceId) | ||
if spaceIds.count == 1 { | ||
window.spaceId = spaceIds.first! | ||
window.spaceIndex = Spaces.idsAndIndexes.first { $0.0 == spaceIds.first! }!.1 | ||
|
@@ -298,6 +300,8 @@ class Windows { | |
window.spaceIndex = Spaces.currentSpaceIndex | ||
window.isOnAllSpaces = true | ||
} | ||
debugPrint("44|",window.cgWindowId!,spaceIds,window.spaceId,window.title!) | ||
|
||
} | ||
} | ||
|
||
|
@@ -345,14 +349,23 @@ class Windows { | |
} | ||
|
||
private static func refreshIfWindowShouldBeShownToTheUser(_ window: Window) { | ||
window.shouldShowTheUser = | ||
var isSameApp = false | ||
if let a = window.application.runningApplication.bundleIdentifier{ | ||
if let b = NSWorkspace.shared.frontmostApplication?.bundleIdentifier{ | ||
if a.contains(b) || b.contains(a) { | ||
isSameApp = true | ||
} | ||
} | ||
} | ||
window.shouldShowTheUser = window.isNormal && | ||
Comment on lines
+352
to
+360
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 please share why you added this check? Currently we compare the pid of the 2 apps. It seems it's not enough, and you added another check of bundleIds. Could you please share in which cases checking the pids was not working? To my knowledge, every app has a pid, but not every app has a bundleID. So in that sense, checking pid should be broader, and sufficient. 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. When some apps start windows, they may add a suffix to the bundle. you can use lark(https://www.larksuite.com/) and start a meeting window and test it |
||
!(window.application.bundleIdentifier.flatMap { id in | ||
Preferences.blacklist.contains { | ||
id.hasPrefix($0.bundleIdentifier) && | ||
($0.hide == .always || (window.isWindowlessApp && $0.hide != .none)) | ||
} | ||
} ?? false) && | ||
!(Preferences.appsToShow[App.app.shortcutIndex] == .active && window.application.pid != NSWorkspace.shared.frontmostApplication?.processIdentifier) && | ||
!(Preferences.appsToShow[App.app.shortcutIndex] == .active && | ||
(window.application.pid != NSWorkspace.shared.frontmostApplication?.processIdentifier && !isSameApp)) && | ||
!(!(Preferences.showHiddenWindows[App.app.shortcutIndex] != .hide) && window.isHidden) && | ||
((!Preferences.hideWindowlessApps && window.isWindowlessApp) || | ||
!window.isWindowlessApp && | ||
|
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.
We alread check this in the
isActualWindow
method, which is used before calling this constructor. So I think rechecking here is not necessary.Perhaps you saw something that I'm missing?
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.
This method(isActualWindow) is not perfect, so use lark to open a metting window. There will be a forced front
and small meeting window in the upper right corner. It is meaningless to switch to this window.
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.
isActualWindow
checks if the window is reasonable to list in our switcher. It should be either a standard-window, or a dialog. It should have a title, a size bigger than a minimum size, etc.Some apps are incorrectly coded, and they will end up having their windows either show up when they should't show up, or not show when they should show.
We've hard-coded some exceptions over the years. These days I don't want to hard-code any exception. It's on the broken apps to fix themselves. It's not on AltTab to add magic code to repair specific apps. Furthermore, these apps change quickly, so our workarounds may break things later.
It's definitely on the broken apps to fix themselves. Their broken code affects AltTab and it also affects other accessibility apps.
Here this lark is most likely having an issue. I suggest you report it to their support, so they get it fixed.
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 fact is that lark does not adapt to this software, but as a developer, I can understand your considerations. I accept it.