Force FL window foreground when switching keyboard layout #3375
Conversation
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
|
Warning Rate limit exceeded@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Helper as Win32Helper
participant PInvoke as PInvoke
App->>Helper: Call SwitchToEnglishKeyboardLayout()
Helper->>Helper: Retrieve main window handle via GetWindowHandle()
Helper->>Helper: Check if handle is the foreground window (IsForegroundWindow)
alt Window is not foreground
Helper->>PInvoke: Invoke SetForegroundWindow(hwnd)
PInvoke-->>Helper: Return status
alt Operation fails
Helper-->>App: Throw Win32Exception
else Operation succeeds
Helper-->>App: Continue execution
end
else Window is already foreground
Helper-->>App: Continue execution
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request forces the FL main window to be the foreground window when switching keyboard layouts. Key changes include:
- Added two overloaded methods to check if a window is the foreground window.
- Modified SwitchToEnglishKeyboardLayout to obtain the FL main window handle and bring it to the foreground.
- Renamed the loop variable in the IME language tag check for clarity.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that when switching keyboard layouts, the FL main window is forced into the foreground.
- Added two helper methods to check if a window or a handle is the foreground window.
- Updated the keyboard layout switching logic to retrieve and force the FL main window to the foreground before proceeding.
| @@ -124,6 +124,16 @@ public static bool SetForegroundWindow(nint handle) | |||
| return PInvoke.SetForegroundWindow(new(handle)); | |||
| } | |||
|
|
|||
| public static bool IsForegroundWindow(Window window) | |||
| { | |||
| return GetWindowHandle(window).Equals(PInvoke.GetForegroundWindow()); | |||
There was a problem hiding this comment.
let call IsForegroundWindow with the handle from GetWindowHandle
There was a problem hiding this comment.
Cannot get your idea. I already have a internal version which uses the handle from GetWindowHandle.
internal static bool IsForegroundWindow(HWND handle)
{
return handle.Equals(PInvoke.GetForegroundWindow());
}
There was a problem hiding this comment.
Since HWND is an internal class, we cannot use that internal class in public function
There was a problem hiding this comment.
huh? You can. Just do IsForegroundWindow(GetWindowHandle(window))
There was a problem hiding this comment.
I think you misunderstand how the modifier works. It won't restrict which method it can call inside the body, but only whether other can call it.
There was a problem hiding this comment.
But it gives me error here:
There was a problem hiding this comment.
sorry for my misunderstanding, now I use GetWindowHandle inside its function
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
387-391: Consider optimizing the language tag check.The loop calls
GetLanguageTag(currentLangId)on each iteration, butcurrentLangIddoesn't change within the loop. This is inefficient.var currentLangId = (uint)currentLayout.Value & KeyboardLayoutLoWord; +var currentLangTag = GetLanguageTag(currentLangId); foreach (var imeLangTag in ImeLanguageTags) { - var langTag = GetLanguageTag(currentLangId); - if (langTag.StartsWith(imeLangTag, StringComparison.OrdinalIgnoreCase)) return; + if (currentLangTag.StartsWith(imeLangTag, StringComparison.OrdinalIgnoreCase)) return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Infrastructure/Win32Helper.cs(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
Flow.Launcher.Infrastructure/PInvokeExtensions.cs (1)
PInvoke(8-25)Flow.Launcher/MainWindow.xaml.cs (2)
MainWindow(30-1034)MainWindow(70-82)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (3)
Flow.Launcher.Infrastructure/Win32Helper.cs (3)
127-130: The implementation looks good.The method provides a useful public API for WPF applications to check if a Window is the foreground window, delegating to the internal implementation properly.
132-135: The implementation is correct.This internal method provides the core implementation for checking if a window handle is the foreground window, which is exactly what was discussed in the previous review comments.
367-376: Foreground window check is now properly implemented.Good implementation of getting the main window handle and ensuring it's the foreground window before proceeding with the keyboard layout switch. This addresses the main objective of the PR to force the FL window into foreground when switching keyboard layouts.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This pull request ensures that the Flow Launcher (FL) main window is forced to the foreground when switching the keyboard layout.
- Added new overloads for IsForegroundWindow to validate if a given Window or HWND is in the foreground.
- Updated SwitchToEnglishKeyboardLayout to use the FL main window and to handle focus changes by calling SetForegroundWindow if needed.
- Revised the IME language tag checking loop to streamline condition evaluation.
|
@Yusyuriv Are we happy to merge this? |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
Follow on with #3366.
When switching keyboard layout, we need to make sure foreground window is FL main window.