Skip to content

Comments

feat(components-native): expose onLoadEnd from FormatFileThumbnail#2926

Open
YousufAJ wants to merge 4 commits intomasterfrom
JOB-153532/extend-atlantis-format-file-thumbnail-to-access-on-load-end
Open

feat(components-native): expose onLoadEnd from FormatFileThumbnail#2926
YousufAJ wants to merge 4 commits intomasterfrom
JOB-153532/extend-atlantis-format-file-thumbnail-to-access-on-load-end

Conversation

@YousufAJ
Copy link
Contributor

@YousufAJ YousufAJ commented Feb 24, 2026

Motivations

Exposing OnLoadEnd method on FormatFileThumbnail to facilitate tracking metrics.

Changes

Exponse OnLoadEnd from FormatFileThumbnail

Changes can be
tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 24, 2026

Deploying atlantis with  Cloudflare Pages  Cloudflare Pages

Latest commit: ca2b4c2
Status: ✅  Deploy successful!
Preview URL: https://d34c3dbd.atlantis.pages.dev
Branch Preview URL: https://job-153532-extend-atlantis-f.atlantis.pages.dev

View logs

/**
* A function to be called when the image has loaded.
*/
readonly onLoadEnd?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

onLoadEnd is currently only passed to the MediaView and not the FileView which makes sense but I think we could improve the prop name and documentation accordingly. I'm think onMediaLoadEnd with the docstring mentioning that it is only called when loading media as opposed to files.

/**
* A function to be called when the image has loaded.
*/
readonly onLoadEnd?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be worth marking this prop as @internal and mentioning that it is only called by FormatFileThumbnail in the docstring

@@ -18,6 +18,7 @@ interface MediaViewProps {
readonly file: FormattedFile;
Copy link
Contributor

Choose a reason for hiding this comment

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

What will the observability logic on your side look like? If this is being added to gain observability into load times, what will you be using for the start time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a rough branch I can push if you'd like to see more details but basically was just planning to have a useEffect for when the page renders to start the timer. We then will load 16 images, and increment a counter when each of the images onLoadEnd is called. When we hit 16 on the counter that will be our total load time.

We are looking to make some optimizations for loading the thumbnail instead of the full image and want to put some numbers too it. Eventually we would also like to add some tracking using OnLoadEnd to help us figure out how much an ImageCache would help with load times as well, as we saw some poor performance on specifically android devices!"

Hope that makes sense and happy to answer any more questions!

@YousufAJ YousufAJ requested a review from Aiden-Brine February 24, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants