-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat(bloc_concurrency): add visual example app #4551
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
feat(bloc_concurrency): add visual example app #4551
Conversation
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.
Thanks for the contribution! This looks great 💙
I'll take a closer look and try to get this merged later today.
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.
Self review done
Sure, thanks @felangel . |
Hi @felangel ,I added test cases for this bloc_concurrency example. Can you please review it too? |
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.
Test cases self-review done
Sorry for the delay! This is next on my list of PRs to review (should have some time tomorrow). |
…pendency from pubspec.yaml
…concurrency' into kiran/example/bloc_concurrency
Hello @felangel , I updated docs too. It would be great if you could review this PR ? and if any changes are required, kindly suggest me. Thanks! |
Thanks so much! Yup it’s next on my list of things to do. I’ll get it merged by this weekend, apologies for the delay! |
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.
Thanks for taking the time to make this contribution! I took a look and I think we should try to simplify the demo just to make it easier to maintain and to reduce the complexity while still conveying the same information. Since bloc_concurrency was heavily inspired by ember concurrency, what are your thoughts on refactoring the example to be more inline with the ember concurrency demos at https://ember-concurrency.com/docs/task-concurrency/
I think the bridge tab is currently a bit too complex with lots of magic numbers in code like:
// Animated balls on bridge
...state.ballsOnBridge.map((ball) {
final screenWidth =
MediaQuery.of(context).size.width -
32 -
50; // Account for padding and platforms
final leftPosition =
25 +
(ball.position *
(screenWidth -
50)); // 25 = start platform width
I'd very much prefer to keep examples as simple and easy to maintain as possible and while I think your initiative of making an example that demonstrates bloc_concurrency is a great one, I think the current example has a lot of added complexity that we can eliminate. Let me know what you think and thanks again for taking the time to contribute!
Sure thank you @felangel ,I will check this demo and will refactor the code. |
Thanks so much! Going to close this for now and look forward to reviewing the simplified example 🙏 |
Status
READY
Breaking Changes
NO
Description
Added movable ball demo example by considering the below transformers provided by bloc_concurrency
Type of Change