-
-
Notifications
You must be signed in to change notification settings - Fork 448
Fixed Progressbar Dispatcher Exception #1445
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
Fixed Progressbar Dispatcher Exception #1445
Conversation
Will this make the progressbar always run in the background? people has complaint about the tiny little cpu usage because of that before. |
I only changed the storyboard binding. so it doesn't matter. |
basically discussion here #295. (Although I think it is a very trivial cost to have it always animate) |
Sorry I just saw that you use trigger to control the animation. That's a much better solution. Thank you so much. |
@taooceros have you reviewed this and good to merge? |
sorry I thought I already merged... |
I just give a similar test for that. There's tiny little cpu usage when flow is hide in this pr compared to the current dev branch (only <.5%). |
Hmm, what steps did you do to test it, let me give it a test. |
Also @Sparrkle @taooceros how do I reproduce this error consistently? |
@Sparrkle I add stopStoryBoard when exit the trigger. Could you please take a look on whether I am doing it correctly? |
@taooceros @Sparrkle is this PR still required? Still getting that error? |
It's a better implementation so let's get it in anyway. Though we may need to resolve the conflict |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe changes involve updates to the main window's visual and animation logic. In the XAML file, a new Changes
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 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 (
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/MainWindow.xaml.cs (1)
430-473
: Excellent refactoring of progress bar animation!This implementation significantly improves the progress bar animation logic by:
- Using a local storyboard variable instead of a persistent field
- Implementing a visibility-based trigger system that automatically starts/stops animation
- Freezing animation objects for better performance
- Properly structuring the style with animation triggers
This addresses the dispatcher exception mentioned in the PR by removing any dependency on persistent state tracking and instead using the WPF trigger system to manage animation lifecycle automatically.
Consider adding a brief comment explaining the animation's purpose and behavior, like:
private void InitProgressbarAnimation() { + // Sets up an infinite animation for the progress bar that automatically + // starts/stops based on the progress bar's visibility var progressBarStoryBoard = new Storyboard(); // Rest of code...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/MainWindow.xaml.cs
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (2)
Flow.Launcher/MainWindow.xaml.cs (2)
344-346
: LGTM for menu item formatting.Proper formatting of menu items with proper translations and icons.
362-364
: LGTM for exit menu item formatting.Proper formatting with translations and icon assignment.
This comment has been minimized.
This comment has been minimized.
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.
Look good! Thanks for your idea.
@Sparrkle @taooceros Does this work for you? |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
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)
If the flagged items are 🤯 false positivesIf items relate to a ...
|
Removed Dispatcher and change how it works storyboard.
What's the PR
Tested
StartLoadingBar
&StopLoadingBar
can work.