Skip to content
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

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

Sparrkle
Copy link
Contributor

@Sparrkle Sparrkle commented Oct 5, 2022

Removed Dispatcher and change how it works storyboard.

What's the PR

  • broken progressbar sometimes.

unknown

@taooceros
Copy link
Member

Will this make the progressbar always run in the background? people has complaint about the tiny little cpu usage because of that before.

@Sparrkle
Copy link
Contributor Author

Sparrkle commented Oct 6, 2022

I only changed the storyboard binding. so it doesn't matter.

@taooceros
Copy link
Member

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)

@taooceros
Copy link
Member

Sorry I just saw that you use trigger to control the animation. That's a much better solution. Thank you so much.

@jjw24
Copy link
Member

jjw24 commented Nov 14, 2022

@taooceros have you reviewed this and good to merge?

@taooceros
Copy link
Member

@taooceros have you reviewed this and good to merge?

sorry I thought I already merged...

@taooceros
Copy link
Member

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%).

@jjw24
Copy link
Member

jjw24 commented Nov 23, 2022

Hmm, what steps did you do to test it, let me give it a test.

@jjw24
Copy link
Member

jjw24 commented Nov 23, 2022

Also @Sparrkle @taooceros how do I reproduce this error consistently?

@taooceros
Copy link
Member

@Sparrkle I add stopStoryBoard when exit the trigger. Could you please take a look on whether I am doing it correctly?

@jjw24
Copy link
Member

jjw24 commented Dec 27, 2022

@taooceros @Sparrkle is this PR still required? Still getting that error?

@taooceros
Copy link
Member

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants