Skip to content

Conversation

@johankasperi
Copy link
Contributor

@johankasperi johankasperi commented Oct 14, 2025

PR Description

Adds support for https://developer.apple.com/documentation/uikit/uitabbarcontroller/bottomaccessory by implementing this prop:

renderBottomAccessory={(props: { placement: "none" | "inline" | "expanded" }) => React.ReactElement}

How to test?

Use the new example in the example app named "Bottom Accessory View".

Screenshots

bottom-accessory-view.mov

@changeset-bot
Copy link

changeset-bot bot commented Oct 14, 2025

🦋 Changeset detected

Latest commit: d02504c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@johankasperi
Copy link
Contributor Author

johankasperi commented Oct 17, 2025

@okwasniewski any chance you could take a look on this? Thank you!

@okwasniewski okwasniewski requested a review from Copilot October 18, 2025 10:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds iOS bottom accessory view support by exposing a renderBottomAccessoryView prop and wiring it to a new native component and SwiftUI integration.

  • Introduces renderBottomAccessoryView prop to render a bottom accessory view on iOS.
  • Adds a new native component (BottomAccessoryView) and iOS component view (RCTBottomAccessoryComponentView) with SwiftUI modifier to attach as TabView bottom accessory.
  • Updates docs and example app to demonstrate usage.

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/react-native-bottom-tabs/src/TabView.tsx Exposes renderBottomAccessoryView prop and conditionally renders BottomAccessoryView on iOS.
packages/react-native-bottom-tabs/src/BottomAccessoryViewNativeComponent.ts Declares the native component interface via codegen.
packages/react-native-bottom-tabs/src/BottomAccessoryView.tsx Wraps the native component, tracks layout and placement, and renders user content.
packages/react-native-bottom-tabs/package.json Registers BottomAccessoryView iOS component provider.
packages/react-native-bottom-tabs/ios/TabView/NewTabView.swift Integrates SwiftUI tabViewBottomAccessory and emits placement changes.
packages/react-native-bottom-tabs/ios/RCTBottomAccessoryComponentView.mm Implements the Fabric view and event emission for layout and placement.
packages/react-native-bottom-tabs/ios/RCTBottomAccessoryComponentView.h Declares the Fabric view interface.
docs/docs/docs/guides/usage-with-react-navigation.mdx Documents the new renderBottomAccessoryView prop.
docs/docs/docs/guides/standalone-usage.md Documents the new renderBottomAccessoryView prop.
apps/example/src/Examples/BottomAccessoryView.tsx Example demonstrating bottom accessory usage.
apps/example/src/App.tsx Adds the new example to the example app menu.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +189 to +195
/**
* A function that returns a React element to display as bottom accessory view.
* iOS 26+ only.
*
* @platform ios
*/
renderBottomAccessoryView?: BottomAccessoryViewProps['renderBottomAccessoryView'];
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The docstring states 'iOS 26+ only', which doesn't exist. Update to the actual minimum supported version (e.g., 'iOS 18+ only') to match platform availability.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@okwasniewski okwasniewski left a comment

Choose a reason for hiding this comment

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

Thank you for contributing this! This is awesome 🚀

I've tested it and it works great, just few small comments

);
})}
{Platform.OS === 'ios' &&
parseFloat(Platform.Version) >= 26 &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this version check? I guess it might not be enough anyways as an app can run on iOS 26 compiled with old Xcode or with a flag that's opting out of new design.

We should probably render it anyways and handle it on the native side

Copy link
Contributor Author

@johankasperi johankasperi Oct 20, 2025

Choose a reason for hiding this comment

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

I thought it would be better not to render the view on the JS side at all on <iOS26 and Android because it wont be visible at all on those platforms?

var placementValue = "none"
if (tabViewBottomAccessoryPlacement == .inline) {
placementValue = "inline"
} else if (tabViewBottomAccessoryPlacement == .expanded) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} else if (tabViewBottomAccessoryPlacement == .expanded) {
} else if tabViewBottomAccessoryPlacement == .expanded {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 114 to 115
let selectorString = "emitOnPlacementChanged:"
let selector = NSSelectorFromString(selectorString)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be solved by a shared protocol between RCTBottomAccessoryView and NewTabView this way we would have type safety

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I will do that asap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The commit above introduced a bug making it impossible to press pressable items in the accessory view. So I need to look into that. Do you know what could have caused this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is adding the BottomAccessoryProvider as contentView to RCTBottomAccessoryComponentView that causes this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved it in 8dfd717

@johankasperi
Copy link
Contributor Author

Thank you for contributing this! This is awesome 🚀

I've tested it and it works great, just few small comments

Glad you liked it and thanks for the review! Responded to some of your comments and will commit fixes to the others asap

cursor[bot]

This comment was marked as outdated.

@johankasperi
Copy link
Contributor Author

@okwasniewski I think I've made all the changes you requested or responded to your comments. Would be awesome if you could have a look and review it again. Thank you!

cursor[bot]

This comment was marked as outdated.

@okwasniewski
Copy link
Collaborator

@johankasperi Thanks for applying my review comments. My biggest concern now is the asynchronous nature of how we emit size events. For views that get sized once I think it's fine to do it but this one gets frequently resized. I think we would need to use shadow nodes for this and update the state using the newly introduced flag (https://github.com/facebook/react-native/blob/b8463053d4cac0c36d6bd54359280be827250aed/packages/react-native/ReactCommon/react/renderer/core/EventQueue.h#L32-L37).

Maybe we can land it as unstable (or just don't mention it in docs) for now and improve it before making it a public API.

@johankasperi
Copy link
Contributor Author

@johankasperi Thanks for applying my review comments. My biggest concern now is the asynchronous nature of how we emit size events. For views that get sized once I think it's fine to do it but this one gets frequently resized. I think we would need to use shadow nodes for this and update the state using the newly introduced flag (https://github.com/facebook/react-native/blob/b8463053d4cac0c36d6bd54359280be827250aed/packages/react-native/ReactCommon/react/renderer/core/EventQueue.h#L32-L37).

Maybe we can land it as unstable (or just don't mention it in docs) for now and improve it before making it a public API.

Ok! I see what you mean but unfortunately I'm not that familiar with this flag or building shadow nodes, could you assist me with that? Feel free to commit to this PR or as you say we could improve it in future PRs


- Type: `ColorValue`

#### `renderBottomAccessoryView` <Badge text="iOS" type="info" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add <Badge text="experimental" type="danger"/> here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

);
}
```
#### `renderBottomAccessoryView` <Badge text="iOS" type="info" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here let's add here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +48 to +51
eventEmitter->onNativeLayout(BottomAccessoryViewEventEmitter::OnNativeLayout {
.height = frame.size.height,
.width = frame.size.width
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a TODO here, TODO: Rewrite this to emit synchronous layout events using shadow nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 55 to 58
- (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &)oldProps
{
[super updateProps:props oldProps:oldProps];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@okwasniewski
Copy link
Collaborator

Alright, let's land it as experimental - I'll work on the shadow node part

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@johankasperi
Copy link
Contributor Author

johankasperi commented Oct 24, 2025

Alright, let's land it as experimental - I'll work on the shadow node part

Thanks for the review! I've made the changes requested. And thank you for looking into refactor to a shadow node

Copy link
Collaborator

@okwasniewski okwasniewski left a comment

Choose a reason for hiding this comment

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

Thanks for working on it! 🙏

@okwasniewski okwasniewski merged commit e419b58 into callstackincubator:main Oct 24, 2025
6 checks passed
@johankasperi
Copy link
Contributor Author

Thanks for working on it! 🙏

Thank you for the review and support!

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