Skip to content

Conversation

@mike-v2
Copy link
Contributor

@mike-v2 mike-v2 commented Nov 30, 2023

Improves image_picker_for_web README example and updates it to use code excerpts.

Part of flutter/flutter#102679

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@mike-v2
Copy link
Contributor Author

mike-v2 commented Nov 30, 2023

  • Tweaked the README code a tiny bit to make it more testable.
  • Added a small image (from flutter_image/example/assets) in order to test creating an Image widget from an XFile's path.

@mike-v2
Copy link
Contributor Author

mike-v2 commented Nov 30, 2023

await XFile.readAsBytes() seems to be causing an issue on web. It's returning a valid Uint8List but for some reason it causes the test to stall indefinitely once it reaches the next await or the end of the test.

testWidgets('getImageFromPath loads image from XFile path',
(WidgetTester tester) async {
// Create an XFile using the test image path.
final XFile pickedFile = XFile('assets/flutter-mark-square-64.png');
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC, you can't read asset files in tests on web, and this would need to be a blob instead on web. @ditman Am I remembering that correctly?

Copy link
Member

Choose a reason for hiding this comment

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

@mike-v2
Copy link
Contributor Author

mike-v2 commented Jan 9, 2024

Even after creating a blob instead of loading an asset, I'm still having the same issue as before with XFile.readAsBytes(). I can get the test to pass without errors if I replace
image = Image.memory(await pickedFile.readAsBytes());
with this:
final Uint8List bytes = Uint8List.fromList(<int>[0, 1, 2, 3, 4, 5]);
image = Image.memory(bytes);

I'm wondering if it's related to this issue?
It's also very possible I'm making a dumb mistake!

@stuartmorgan-g
Copy link
Collaborator

@ditman Any thoughts on how to structure this test?

@stuartmorgan-g
Copy link
Collaborator

@ditman Ping on the test question above.

@ditman
Copy link
Member

ditman commented Apr 12, 2024

Looking!

@ditman
Copy link
Member

ditman commented Apr 12, 2024

I moved the test to integration_test and added the missing IntegrationTestWidgetsFlutterBinding.ensureInitialized()... I also did other changes (to ensure that the PNG image is a valid one), but I'm not sure if those were needed. Now the tests run and pass!

@stuartmorgan if you're happy with the new contents of the README, I think this could land.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with one fix.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 15, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 15, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Apr 15, 2024

auto label is removed for flutter/packages/5523, due to - The status or check suite Linux_android android_platform_tests_shard_3 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan-g
Copy link
Collaborator

The presubmit failure is OOB breakage I just landed a fix for; once that is published I'll re-run.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 15, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 15, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Apr 15, 2024

auto label is removed for flutter/packages/5523, due to - The status or check suite Linux_android android_platform_tests_shard_3 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 15, 2024
@auto-submit auto-submit bot merged commit 9a24904 into flutter:main Apr 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 16, 2024
flutter/packages@6698b2d...90c876d

2024-04-16 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.24.10 to 3.25.0 (flutter/packages#6545)
2024-04-16 [email protected] [CupertinoIcons] fix broken icons and vertical alignment, bump the version to 1.08 (flutter/packages#6520)
2024-04-16 [email protected] [video_player] Calls `onDestroy` instead of `initialize` in onDetachedFromEngine (flutter/packages#6501)
2024-04-15 [email protected] [image_picker] Adopt code excerpts in README (flutter/packages#5523)
2024-04-15 [email protected] [url_launcher][web] Link should work when triggered by keyboard (flutter/packages#6505)
2024-04-15 [email protected] Manual roll Flutter from 53cba24 to 2e748e8 (19 revisions) (flutter/packages#6541)
2024-04-15 [email protected] Update local_auth_android minSdkVersion to 19 (flutter/packages#6537)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
flutter/packages@6698b2d...90c876d

2024-04-16 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.24.10 to 3.25.0 (flutter/packages#6545)
2024-04-16 [email protected] [CupertinoIcons] fix broken icons and vertical alignment, bump the version to 1.08 (flutter/packages#6520)
2024-04-16 [email protected] [video_player] Calls `onDestroy` instead of `initialize` in onDetachedFromEngine (flutter/packages#6501)
2024-04-15 [email protected] [image_picker] Adopt code excerpts in README (flutter/packages#5523)
2024-04-15 [email protected] [url_launcher][web] Link should work when triggered by keyboard (flutter/packages#6505)
2024-04-15 [email protected] Manual roll Flutter from 53cba24 to 2e748e8 (19 revisions) (flutter/packages#6541)
2024-04-15 [email protected] Update local_auth_android minSdkVersion to 19 (flutter/packages#6537)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
Improves image_picker_for_web README example and updates it to use code excerpts.

Part of [flutter/flutter#102679](flutter/flutter#102679)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: image_picker platform-web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants