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

fix(iOS): buggy search bar / large title behaviour on Fabric #1825

Merged
merged 28 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f991e54
Reinstall pods in FTE
kkafar Jun 28, 2023
a15d63f
Stash various changes (POSSIBLY TO REVERT)
kkafar Jul 6, 2023
d8a83a5
Partially working non react-native setup
kkafar Jul 7, 2023
32a8d9a
Fully working non react-native setup
kkafar Jul 7, 2023
cd6153d
Stash experiments
kkafar Jul 11, 2023
44620d2
Update example in FTE
kkafar Jul 14, 2023
66005ae
set headerTranslucent & largeTitle in TE
kkafar Jul 14, 2023
5ba6d42
Fix on JS side
kkafar Jul 18, 2023
73dde0e
Stash various native changes (to revert)
kkafar Jul 18, 2023
612d158
Restore unnecessary changes in RNSScreen.mm
kkafar Jul 18, 2023
8b0ccf6
Remove unnecessary comment
kkafar Jul 18, 2023
e6bbb0c
Restore App.js in TE
kkafar Jul 18, 2023
5adccc8
Restore changes in navigation container
kkafar Jul 18, 2023
dcdf94b
Remove experiments code
kkafar Jul 18, 2023
d3dd5f4
Remove unused imports in ScreenStack
kkafar Jul 18, 2023
aa6f139
Restore more changes in RNSScreenStack
kkafar Jul 18, 2023
f829aef
Restore changes in RNSScreenStackHeaderConfig
kkafar Jul 18, 2023
e3a6db1
Restore changes in FTE App.js
kkafar Jul 18, 2023
9d78701
Restore some changes in Test1097 in FTE
kkafar Jul 18, 2023
529d554
Merge branch 'main' into @kkafar/searchbar-fabric
kkafar Jul 18, 2023
8679e03
Update & merge main
kkafar Jul 18, 2023
5a7bc7a
Restore more changes in Test1097 FTE
kkafar Jul 18, 2023
a650c08
Replace logic relying on config position in reactSubviews array
kkafar Jul 18, 2023
0ddb965
Refactor hideHeaderIfNecessary once more
kkafar Jul 18, 2023
794861f
Add some comments
kkafar Jul 18, 2023
b7beb2f
Fix `findHeaderConfig` impl on Paper
kkafar Jul 18, 2023
037a348
Add missing comment on why `collapsable={false}` is set on `Container`
kkafar Jul 19, 2023
d19fb23
Add comment on `HeaderConfig` placement
kkafar Jul 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions FabricTestExample/src/Test1097.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import * as React from 'react';
import {Button, NativeSyntheticEvent, ScrollView} from 'react-native';
import {
Expand Down Expand Up @@ -91,10 +90,8 @@ function First({navigation}: NativeStackScreenProps<ParamListBase>) {
onPress={() => searchBarRef.current?.focus()}
/>
{items
.filter(
(item) => item.toLowerCase().indexOf(search.toLowerCase()) !== -1,
)
.map((item) => (
.filter(item => item.toLowerCase().indexOf(search.toLowerCase()) !== -1)
.map(item => (
<Button
title={item}
key={item}
Expand Down
2 changes: 1 addition & 1 deletion TestsExample/src/Test1097.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
/* eslint-disable react-hooks/exhaustive-deps */
import * as React from 'react';
import {Button, NativeSyntheticEvent, ScrollView} from 'react-native';
import {
Expand Down
9 changes: 8 additions & 1 deletion ios/RNSScreen.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ NS_ASSUME_NONNULL_BEGIN

@end

@class RNSScreenStackHeaderConfig;

@interface RNSScreenView :
#ifdef RCT_NEW_ARCH_ENABLED
RCTViewComponentView
Expand Down Expand Up @@ -88,7 +90,8 @@ NS_ASSUME_NONNULL_BEGIN
// we recreate the behavior of `reactSetFrame` on new architecture
@property (nonatomic) facebook::react::LayoutMetrics oldLayoutMetrics;
@property (nonatomic) facebook::react::LayoutMetrics newLayoutMetrics;
@property (weak, nonatomic) UIView *config;
@property (weak, nonatomic) RNSScreenStackHeaderConfig *config;
@property (nonatomic, readonly) BOOL hasHeaderConfig;
#else
@property (nonatomic, copy) RCTDirectEventBlock onAppear;
@property (nonatomic, copy) RCTDirectEventBlock onDisappear;
Expand All @@ -109,12 +112,16 @@ NS_ASSUME_NONNULL_BEGIN
- (void)notifyDisappear;
- (void)updateBounds;
- (void)notifyDismissedWithCount:(int)dismissCount;
- (instancetype)initWithFrame:(CGRect)frame;
kkafar marked this conversation as resolved.
Show resolved Hide resolved
#endif

- (void)notifyTransitionProgress:(double)progress closing:(BOOL)closing goingForward:(BOOL)goingForward;
- (void)notifyDismissCancelledWithDismissCount:(int)dismissCount;
- (BOOL)isModal;

/// Looks for header configuration in instance's `reactSubviews` and returns it. If not present returns `nil`.
kkafar marked this conversation as resolved.
Show resolved Hide resolved
- (RNSScreenStackHeaderConfig *_Nullable)findHeaderConfig;

@end

@interface UIView (RNSScreen)
Expand Down
28 changes: 20 additions & 8 deletions ios/RNSScreen.mm
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ - (instancetype)initWithFrame:(CGRect)frame
_reactSubviews = [NSMutableArray new];
[self initCommonProps];
}

return self;
}
#endif // RCT_NEW_ARCH_ENABLED
Expand Down Expand Up @@ -531,6 +530,16 @@ - (BOOL)isModal
return self.stackPresentation != RNSScreenStackPresentationPush;
}

- (RNSScreenStackHeaderConfig *_Nullable)findHeaderConfig
{
for (UIView *view in self.reactSubviews) {
if ([view isKindOfClass:RNSScreenStackHeaderConfig.class]) {
return (RNSScreenStackHeaderConfig *)view;
}
}
return nil;
}

#if !TARGET_OS_TV
/**
* Updates settings for sheet presentation controller.
Expand Down Expand Up @@ -588,19 +597,24 @@ - (void)updatePresentationStyle
#pragma mark - Fabric specific
#ifdef RCT_NEW_ARCH_ENABLED

- (BOOL)hasHeaderConfig
{
return _config != nil;
}

+ (facebook::react::ComponentDescriptorProvider)componentDescriptorProvider
{
return facebook::react::concreteComponentDescriptorProvider<facebook::react::RNSScreenComponentDescriptor>();
}

- (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
{
[super mountChildComponentView:childComponentView index:index];
if ([childComponentView isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
_config = childComponentView;
((RNSScreenStackHeaderConfig *)childComponentView).screenView = self;
_config = (RNSScreenStackHeaderConfig *)childComponentView;
_config.screenView = self;
}
[_reactSubviews insertObject:childComponentView atIndex:index];
[super mountChildComponentView:childComponentView index:index];
}

- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
Expand Down Expand Up @@ -1194,12 +1208,10 @@ - (void)hideHeaderIfNecessary

// we need to check whether reactSubviews array is empty, because on Fabric child nodes are unmounted first ->
// reactSubviews array may be empty
if (currentIndex > 0 && [self.screenView.reactSubviews count] > 0 &&
[self.screenView.reactSubviews[0] isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
RNSScreenStackHeaderConfig *config = [self.screenView findHeaderConfig];
if (currentIndex > 0 && config != nil) {
UINavigationItem *prevNavigationItem =
[self.navigationController.viewControllers objectAtIndex:currentIndex - 1].navigationItem;
RNSScreenStackHeaderConfig *config = ((RNSScreenStackHeaderConfig *)self.screenView.reactSubviews[0]);

BOOL wasSearchBarActive = prevNavigationItem.searchController.active;

#ifdef RCT_NEW_ARCH_ENABLED
Expand Down
4 changes: 1 addition & 3 deletions ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
#ifdef RCT_NEW_ARCH_ENABLED
#import <React/RCTFabricComponentsPlugins.h>
#import <React/RCTMountingTransactionObserving.h>
#import <React/RCTSurfaceTouchHandler.h>
#import <React/UIView+React.h>
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
#import <react/renderer/components/rnscreens/EventEmitters.h>
#import <react/renderer/components/rnscreens/Props.h>
#import <react/renderer/components/rnscreens/RCTComponentViewHelpers.h>

#import <React/RCTFabricComponentsPlugins.h>

#else
#import <React/RCTBridge.h>
#import <React/RCTRootContentView.h>
Expand Down
9 changes: 8 additions & 1 deletion src/native-stack/views/NativeStackView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ const MaybeNestedStack = ({
]}
// @ts-ignore Wrong props passed to View
stackPresentation={stackPresentation}
// This view must *not* be flattened.
// See https://github.com/software-mansion/react-native-screens/pull/1825
// for detailed explanation.
collapsable={false}
kkafar marked this conversation as resolved.
Show resolved Hide resolved
>
{children}
</Container>
Expand Down Expand Up @@ -331,14 +335,17 @@ const RouteView = ({
isHeaderInPush !== false ? headerHeight : parentHeaderHeight ?? 0
}
>
<HeaderConfig {...options} route={route} headerShown={isHeaderInPush} />
<MaybeNestedStack
options={options}
route={route}
stackPresentation={stackPresentation}
>
{renderScene()}
</MaybeNestedStack>
{/* HeaderConfig must not be first child of a Screen.
See https://github.com/software-mansion/react-native-screens/pull/1825
for detailed explanation */}
<HeaderConfig {...options} route={route} headerShown={isHeaderInPush} />
</HeaderHeightContext.Provider>
</Screen>
);
Expand Down