Skip to content

Conversation

@johankasperi
Copy link
Contributor

@johankasperi johankasperi commented Oct 29, 2025

Description

When using RCTImageLoader for images in headerRightItems/headerLeftItems the asynchronous loadImageWithURLRequest produces som layout bugs as shown under "Screenshots". These are:

  1. When setting title and image the title is briefly visible before the image is set.
  2. When setting only image the bar button item is briefly shown as a very wide item without any content.

Changes

  1. If the image is a ImageRequireSource load the image with [RCTConvert UIImage]. I know its deprecated but thats the only synchronous image loader that I know of.
  2. If the image is a ImageURISource set the bar button title in the completion block of loadImageWithURLRequest
  3. Updates BarButtonItems example to latest types/api from react-navigation/native-stack.

Screenshots / GIFs

Before

Title briefly shown

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2025-10-29.at.21.51.42.mov

Wide bar button item while loadImageWithURLRequest completes

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2025-10-29.at.22.19.46.mov

After

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2025-10-29.at.22.20.18.mov

Test code and steps to reproduce

Use any of the examples with required images in BarButtonItems

Checklist

@kkafar kkafar self-requested a review October 31, 2025 11:23
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Yeah, we have this ugly effect now.

The situation here is a bit more complex.

First: in release mode, the code using RCTImageLoader will work just fine if we remove dispatch_async in completion block -> because this is the source of the delay.

In debug mode, when the assets are not loaded from application bundle (not talking about JS bundle here, rather NSBundle), but are downloaded from packager, RCTImageLoader does this asynchronously, indeed.

Since RCTConvert for image related conversions got deprecated, I'd like to limit its usage only for debug mode, where we can not load images synchronously in any other way. We could do this manually, but I'd like to avoid that option even more.

I'll start conversation with React Native team about giving us support for synchronous loading, since this will be rather a regression if the RCTConvert UIImage gets removed and we don't have any other sync API.

@kkafar
Copy link
Member

kkafar commented Oct 31, 2025

Also disadvantage of using RCTConvert is that we don't have any image caching.

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

I'm drafting a refactor & noticed this. Just a question.

return self;
}

self.title = dict[@"title"];
Copy link
Member

Choose a reason for hiding this comment

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

Why did you resign from setting title here and postponed it for later? Was there a particular reason here?

Now we use RCTConvert in debug mode & RCTImageLoader in release.
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

I've refactored the code a bit. I'll be willing to land this if you confirm that also works for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants