-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Use ResizeImage in ColorScheme._imageProviderToScaled and remove timeout #150560
base: master
Are you sure you want to change the base?
Use ResizeImage in ColorScheme._imageProviderToScaled and remove timeout #150560
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Hey @gawsfbet can you sign the cla? This will likely need a test as well. Thank you for contributing! |
cb16672
to
1e2db4f
Compare
31b840f
to
38d784b
Compare
Already done it, thanks! This PR mainly consists of refactoring a private method, and I have modified existing tests according to the new behavior. Do I need to write additional tests as well? |
…derToScaled method
… 5sec timeout (which should be handled by the caller of ColorScheme.fromImageProvider)
… 5sec timeout (which should be handled by the caller of ColorScheme.fromImageProvider)
… 5sec timeout (which should be handled by the caller of ColorScheme.fromImageProvider)
1bc6cfe
to
cdfe054
Compare
Thanks a lot for the contribution! Yes, if there are any new behaviors, please add unit tests to cover that:) Please let me know once this PR is ready to review! I saw it's still a draft, so not sure whether I should start reviewing😄 |
The timeout case should be handled by the caller of
ColorScheme.fromImageProvider
.Note: the current impl where the listener is removed multiple times might result in the error
Bad state: Stream has been disposed.
, as the stream attempts to remove the listener twice (once during image resolution and once when the timer expires)List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
#124806
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.