Skip to content

Fix hide window startup & Close InvalidOperationException #3390

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

Merged
merged 8 commits into from
Mar 28, 2025

Conversation

Jack251970
Copy link
Contributor

@Jack251970 Jack251970 commented Mar 28, 2025

Fix window flicking during app startup

Follow on with #3388

Test

Open FL and app window is invisible always

@prlabeler prlabeler bot added the bug Something isn't working label Mar 28, 2025
@Jack251970 Jack251970 requested a review from onesounds March 28, 2025 04:02

This comment has been minimized.

Copy link

gitstream-cm bot commented Mar 28, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

@Jack251970 Jack251970 added Dev branch only An issue or fix for the Dev branch build and removed bug Something isn't working labels Mar 28, 2025
@Jack251970 Jack251970 added this to the 1.20.0 milestone Mar 28, 2025
Copy link
Contributor

coderabbitai bot commented Mar 28, 2025

📝 Walkthrough

Walkthrough

This pull request updates the UI behavior and control flow in two areas. In the main window code, the visibility and opacity of the ClockPanel and SearchIcon are adjusted based on query text and plugin icon state. The handling of the first launch is enhanced by showing a WelcomeWindow. In the view model, asynchronous dispatching is simplified by directly managing UI thread access and replacing nested asynchronous calls with direct invocation, streamlining the Show and Hide methods.

Changes

File(s) Change Summary
Flow.Launcher/MainWindow.xaml.cs In the OnLoaded method, additional logic for first launch handling is introduced, setting the backdrop type and showing a WelcomeWindow. The opacity of the ClockPanel and SearchIcon is conditionally managed based on query text and plugin icon state. Opacity values are standardized in OnDeactivated, and a delay is added when animations are enabled. Opacity management is removed from WindowAnimation.
Flow.Launcher/ViewModel/MainViewModel.cs In ChangeQueryTextAsync, asynchronous UI thread invocation is replaced with a direct UI thread access check and asynchronous call if needed. The Show and Hide methods are streamlined by removing unnecessary asynchronous dispatching and directly calling Win32Helper.DWMSetCloakForWindow. Additionally, the signature of Show has been updated from async to a synchronous version.
Flow.Launcher/App.xaml.cs The Log.SetLogLevel(_settings.LogLevel); line is repositioned in the OnStartup method to occur after the pre-start cleanup process, modifying the order of operations during startup without changing functionality.

Possibly related PRs

Suggested labels

enhancement, 10 min review

Suggested reviewers

  • jjw24
  • onesounds

Poem

I'm a rabbit in the code meadow,
Hopping through changes both bright and mellow.
UI panels and icons now dance just right,
With a sound and a focus, shining in the light.
Celebrate the commit with a joyful bunny delight!
🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b519313 and 8269304.

📒 Files selected for processing (1)
  • Flow.Launcher/ViewModel/MainViewModel.cs (4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher/ViewModel/MainViewModel.cs (4)
Flow.Launcher/PublicAPIInstance.cs (1)
  • BackToQueryResults (341-341)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
  • BackToQueryResults (312-312)
Flow.Launcher/Msg.xaml.cs (1)
  • Show (63-91)
Flow.Launcher/MainWindow.xaml.cs (2)
  • MainWindow (30-1079)
  • MainWindow (70-82)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher/ViewModel/MainViewModel.cs (4)

215-216: Added null check to prevent logging during disposal.

The added null check ensures error logs aren't generated when the object is being disposed, which is a good practice for clean shutdown.


641-664: Improved thread handling to prevent UI thread blocking.

This change significantly improves the thread handling logic by:

  1. Using CheckAccess() instead of nested InvokeAsync calls
  2. Only dispatching to the UI thread when necessary
  3. Simplifying the async control flow

The previous implementation likely contributed to window flickering due to inefficient thread management. This approach prevents the UI thread blocking that was causing visibility issues during window transitions.


1451-1455: Key improvement: Using DWM cloaking to properly show the window.

The method signature has been changed from async void to void and now uses the DWMSetCloakForWindow Win32 API to properly handle window visibility. This is a critical change that directly addresses the window flickering issue mentioned in the PR objectives.

The DWM cloaking approach provides a more robust way to control window visibility compared to just using WPF's visibility properties.


1468-1472: Complementary fix: Using DWM cloaking to properly hide the window.

This change mirrors the improvement in the Show() method by using DWMSetCloakForWindow to completely hide the window. The commented line //MainWindowOpacity = 0; suggests that opacity transitions were previously being used but are no longer needed with the DWM cloaking approach.

This implementation provides a cleaner transition when hiding the window, eliminating the flickering effect mentioned in the PR.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@prlabeler prlabeler bot added the bug Something isn't working label Mar 28, 2025
@Jack251970 Jack251970 requested a review from Copilot March 28, 2025 04:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses the window flickering issue during startup by refining how window visibility and UI element properties are handled. Key changes include:

  • Removing unnecessary asynchronous UI updates and recursion in the ChangeQueryTextAsync method.
  • Refactoring the Show and Hide methods to directly work with Application.Current.MainWindow for window cloak handling.
  • Adjusting the opacity and visibility updates for clock and search icon in MainWindow to better support the new startup behavior.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Flow.Launcher/ViewModel/MainViewModel.cs Simplified asynchronous UI updates and window show/hide behavior.
Flow.Launcher/MainWindow.xaml.cs Refactored UI element updates to align with the window startup fix.
Comments suppressed due to low confidence (2)

Flow.Launcher/ViewModel/MainViewModel.cs:641

  • The recursive pattern used here to invoke ChangeQueryText may be risky if the dispatcher thread check fails repeatedly. Please verify that under all conditions the UI thread is eventually accessed to prevent potential infinite recursion.
if (!Application.Current.Dispatcher.CheckAccess())

Flow.Launcher/MainWindow.xaml.cs:809

  • [nitpick] The removal of the UpdatePosition call from the WindowAnimation method may affect the window's positioning during the animation phase. Please confirm that the positioning is handled appropriately elsewhere.
UpdatePosition();

This comment has been minimized.

@Jack251970 Jack251970 removed the bug Something isn't working label Mar 28, 2025
@Jack251970 Jack251970 requested a review from Copilot March 28, 2025 04:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR aims to fix window flickering during app startup by refactoring window show/hide methods and improving dispatcher usage to avoid blocking the UI thread. Key changes include:

  • In MainViewModel.cs, updating ChangeQueryTextAsync to check for UI thread access and refactoring the Show and Hide methods to remove redundant dispatcher invocations.
  • In MainWindow.xaml.cs, modifying the OnLoaded and OnDeactivated handlers to adjust UI element properties and backdrop settings, and updating WindowAnimation to better manage animation timing.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Flow.Launcher/ViewModel/MainViewModel.cs Refactored dispatcher usage in ChangeQueryTextAsync and simplified the Show/Hide methods.
Flow.Launcher/MainWindow.xaml.cs Updated UI element handling during window load/deactivation and adjusted window animation.
Comments suppressed due to low confidence (3)

Flow.Launcher/ViewModel/MainViewModel.cs:641

  • Ensure that the added dispatcher access check in ChangeQueryTextAsync maintains the correct execution context for UI updates. Confirm that the recursive invocation doesn't lead to unintended side effects.
if (!Application.Current.Dispatcher.CheckAccess())

Flow.Launcher/ViewModel/MainViewModel.cs:1453

  • The removal of detailed UI element updates (like opacity and visibility adjustments) in the Show method may lead to visual inconsistencies; verify that calling only DWMSetCloakForWindow achieves the intended result.
Win32Helper.DWMSetCloakForWindow(Application.Current.MainWindow, false);

Flow.Launcher/MainWindow.xaml.cs:815

  • [nitpick] The removal of the UpdatePosition() call prior to starting the window animation might cause position discrepancies; please confirm if updating the position is no longer required for the animation to run smoothly.
var clocksb = new Storyboard();

@prlabeler prlabeler bot added the bug Something isn't working label Mar 28, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Jack251970 Jack251970 requested a review from Copilot March 28, 2025 06:50
Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
❌ forbidden-pattern 24
⚠️ non-alpha-in-dictionary 19

See ❌ Event descriptions for more information.

Forbidden patterns 🙅 (1)

In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.

These forbidden patterns matched content:

s.b. workaround(s)

\bwork[- ]arounds?\b
If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes window flickering and startup invisibility by updating the visibility, opacity, and cloak handling of the application window. Key changes include:

  • Refactoring the Show, Hide, and ChangeQueryTextAsync methods to simplify dispatcher usage and avoid UI thread blocking.
  • Adjusting first-launch settings and updating UI element properties in MainWindow.xaml.cs to improve startup behavior.
  • Removing redundant code and improving consistency in UI updates during deactivation and animation.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Flow.Launcher/ViewModel/MainViewModel.cs Refactored Show/Hide methods and improved dispatcher access check to manage window cloak and avoid flickering.
Flow.Launcher/MainWindow.xaml.cs Updated first launch behaviors, renamed certain UI update sequences, and replaced Close() with Shutdown() for graceful termination.
Comments suppressed due to low confidence (2)

Flow.Launcher/MainWindow.xaml.cs:114

  • [nitpick] Local variable 'WelcomeWindow' should use camelCase (e.g., 'welcomeWindow') to align with standard C# naming conventions.
var WelcomeWindow = new WelcomeWindow();

Flow.Launcher/ViewModel/MainViewModel.cs:1454

  • Consider adding a type check or assertion to ensure that Application.Current.MainWindow is of the expected type before calling DWMSetCloakForWindow to prevent potential runtime errors.
Win32Helper.DWMSetCloakForWindow(Application.Current.MainWindow, false);

Copy link

gitstream-cm bot commented Mar 28, 2025

🥷 Code experts: onesounds

Jack251970, onesounds have most 👩‍💻 activity in the files.
Jack251970, onesounds have most 🧠 knowledge in the files.

See details

Flow.Launcher/MainWindow.xaml.cs

Activity based on git-commit:

Jack251970 onesounds
MAR 748 additions & 770 deletions 327 additions & 141 deletions
FEB 1 additions & 1 deletions 8 additions & 4 deletions
JAN
DEC 5 additions & 10 deletions
NOV
OCT

Knowledge based on git-blame:
Jack251970: 57%
onesounds: 21%

Flow.Launcher/ViewModel/MainViewModel.cs

Activity based on git-commit:

Jack251970 onesounds
MAR 369 additions & 337 deletions 293 additions & 201 deletions
FEB 63 additions & 21 deletions 23 additions & 25 deletions
JAN 17 additions & 21 deletions
DEC 59 additions & 63 deletions
NOV 39 additions & 15 deletions
OCT

Knowledge based on git-blame:
Jack251970: 23%
onesounds: 12%

To learn more about /:\ gitStream - Visit our Docs

@Jack251970 Jack251970 enabled auto-merge March 28, 2025 06:57
@Jack251970 Jack251970 changed the title Fix hide window startup Fix hide window startup & Close InvalidOperationException Mar 28, 2025
@Jack251970 Jack251970 merged commit 0a497d8 into dev Mar 28, 2025
12 checks passed
@Jack251970 Jack251970 deleted the fix_hide_window_startup branch March 28, 2025 07:04
@jjw24 jjw24 removed bug Something isn't working 5 min review labels Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev branch only An issue or fix for the Dev branch build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants