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

[Fabric] Implement onProgress for Image #14493

Merged
merged 6 commits into from
Apr 10, 2025

Conversation

anupriya13
Copy link
Contributor

@anupriya13 anupriya13 commented Apr 5, 2025

Description

Type of Change

  • New feature (non-breaking change which adds functionality)

Why

What is the motivation for this change? Add a few sentences describing the context and overall goals of the pull request's commits.

Implement the onProgress property for the fabric implementation of Image.

Note: At one point there was a TODO for this in our new fabric code.

This property was not available in RNW Paper but could be implemented for Fabric.

See https://reactnative.dev/docs/image#onprogress for details.

Resolves #13118 and #13752

What

What changes were made to the codebase to solve the bug, add the functionality, etc. that you specified above.

Added a progress Callback during request start for image downloading that keeps track of progress and invokes observerCoordinator->nativeImageResponseProgress

Refer:
https://learn.microsoft.com/en-us/uwp/api/windows.web.http.httpstringcontent.writetostreamasync?view=winrt-26100
https://learn.microsoft.com/en-us/uwp/api/windows.web.http.httpstringcontent.readasinputstreamasync?view=winrt-26100

Key Differences between ReadAsInputStreamAsync and WriteToStreamAsync:

Aspect ReadAsInputStreamAsync + Manual Loop WriteToStreamAsync
Control ✅ Full control over buffer size, read/write logic, progress reporting, cancellation ❌ Black-box – you don’t control how reading/writing is done internally
Progress Reporting ✅ You can track progressCallback(progress, total) manually ❌ No built-in way to hook into progress during transfer
Customization ✅ Can add error handling, retries, filters, stalls, compression, etc. ❌ Not customizable, does an internal streaming copy
Code Simplicity ❌ More verbose ✅ One-liner – cleaner if you don’t need control
Performance Slightly more overhead due to manual buffering Likely optimized under the hood, but not significantly different

Screenshots

Add any relevant screen captures here from before or after your changes.

image
image
image

Testing

If you added tests that prove your changes are effective or that your feature works, add a few sentences here detailing the added test scenarios.

  1. Tested in playground composition adding the onProgress event call with large and small size image.
  2. Tested in RNWTester e2e fabric test app, testID: image-network-callback and image-network. 'An Image component can have a network callback' and 'A network Image example'

Optional: Describe the tests that you ran locally to verify your changes.

Changelog

Should this change be included in the release notes: indicate yes or no
Yes

Add a brief summary of the change to use in the release notes for the next release.
Implemented onProgress event for Image in Fabric new architecture.
Invoked on download progress.

Microsoft Reviewers: Open in CodeFlow

@anupriya13 anupriya13 marked this pull request as ready for review April 7, 2025 00:35
@anupriya13 anupriya13 requested a review from a team as a code owner April 7, 2025 00:35
@anupriya13 anupriya13 requested a review from acoates-ms April 7, 2025 02:54
@anupriya13 anupriya13 added Area: Image Area: Fabric Support Facebook Fabric New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric labels Apr 7, 2025
@anupriya13 anupriya13 added this to the Next milestone Apr 7, 2025
@anupriya13 anupriya13 requested a review from TatianaKapos April 8, 2025 09:45
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) and removed Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) labels Apr 9, 2025
@anupriya13 anupriya13 merged commit d449c00 into microsoft:main Apr 10, 2025
60 checks passed
@anupriya13 anupriya13 deleted the user/anuverma/onProgressImage branch April 10, 2025 16:07
@anupriya13 anupriya13 added Parity: Fabric vs. Paper RNW Fabric does not look or behave like RNW Paper Workstream: Component Parity Close the parity gap between RNW and RN for core RN components and their supporting APIs. labels Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Completion Area: Fabric Support Facebook Fabric Area: Image enhancement New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric Parity: Fabric vs. Paper RNW Fabric does not look or behave like RNW Paper Workstream: Component Parity Close the parity gap between RNW and RN for core RN components and their supporting APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement onProgress property for Image for fabric
3 participants