diff --git a/FabricExample/e2e/single-feature-tests/tabs/test-tabs-bottom-accessory-layout-ios.e2e.ts b/FabricExample/e2e/single-feature-tests/tabs/test-tabs-bottom-accessory-layout-ios.e2e.ts index 7e65c46655..625ebde19e 100644 --- a/FabricExample/e2e/single-feature-tests/tabs/test-tabs-bottom-accessory-layout-ios.e2e.ts +++ b/FabricExample/e2e/single-feature-tests/tabs/test-tabs-bottom-accessory-layout-ios.e2e.ts @@ -12,8 +12,8 @@ const bottomAccessoryElement = (testID: string) => by.id(testID).withAncestor(by.type('RNSTabsBottomAccessoryComponentView')), ).atIndex(0); -async function expectBottomAccessoryVisible(testID: string) { - await expect(bottomAccessoryElement(testID)).toBeVisible(); +async function expectBottomAccessoryExist(testID: string) { + await expect(bottomAccessoryElement(testID)).toExist(); } async function expectBottomAccessoryText(testID: string, text: string) { @@ -73,32 +73,32 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { }); it('should show the Upper Left accessory variant on initial load', async () => { - await expectBottomAccessoryVisible('accessory-upper-left'); + await expectBottomAccessoryExist('accessory-upper-left'); await expectBottomAccessoryText('accessory-upper-left', 'Upper Left'); }); it('should update the accessory when Center variant card is tapped', async () => { await element(by.id('variant-center')).tap(); - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); }); it('should update the accessory when Lower Right variant card is tapped', async () => { await element(by.id('variant-lower-right')).tap(); - await expectBottomAccessoryVisible('accessory-lower-right'); + await expectBottomAccessoryExist('accessory-lower-right'); await expectBottomAccessoryText('accessory-lower-right', 'Lower Right'); }); it('should update the accessory when Long variant card is tapped', async () => { await element(by.id('variant-long')).tap(); - await expectBottomAccessoryVisible('accessory-long'); + await expectBottomAccessoryExist('accessory-long'); }); it('should update the accessory when RGB variant card is tapped', async () => { await element(by.id('variant-rgb')).tap(); - await expectBottomAccessoryVisible('rgb-strip-0'); - await expectBottomAccessoryVisible('rgb-strip-1'); - await expectBottomAccessoryVisible('rgb-strip-2'); + await expectBottomAccessoryExist('rgb-strip-0'); + await expectBottomAccessoryExist('rgb-strip-1'); + await expectBottomAccessoryExist('rgb-strip-2'); }); // --------------------------------------------------------------------------- @@ -107,17 +107,17 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { it('should preserve the accessory when switching to the ScrollDown tab and back', async () => { await element(by.id('variant-center')).tap(); - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); await forceTapByLabeliOS('scroll-down-tab-item-label'); await expect(element(by.id('scroll-down-scrollview'))).toBeVisible(); - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); await forceTapByLabeliOS('config-tab-item-label'); await expect(element(by.id('config-scrollview'))).toBeVisible(); - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); }); @@ -140,7 +140,7 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { by.type('UITabBar'), ).getAttributes()) as IosElementAttributes; - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); expectBottomAccessoryExtended(extendedBottomAccessory, extendedTabBar); }); @@ -168,7 +168,7 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { .atIndex(0) .getAttributes()) as IosElementAttributes; - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); expectBottomAccessoryInline( inlineBottomAccessory, @@ -211,7 +211,7 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { by.type('UITabBar'), ).getAttributes()) as IosElementAttributes; - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); expectBottomAccessoryExtended(extendedBottomAccessory, extendedTabBar); }); @@ -239,7 +239,7 @@ describeIfiOS('Tabs bottomAccessory (iOS)', () => { .atIndex(0) .getAttributes()) as IosElementAttributes; - await expectBottomAccessoryVisible('accessory-center'); + await expectBottomAccessoryExist('accessory-center'); await expectBottomAccessoryText('accessory-center', 'Center'); expectBottomAccessoryInline( inlineBottomAccessory, diff --git a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryContentComponentView.mm b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryContentComponentView.mm index e0c87b4b31..ec7cec3520 100644 --- a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryContentComponentView.mm +++ b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryContentComponentView.mm @@ -27,34 +27,19 @@ - (instancetype)initWithFrame:(CGRect)frame - (void)didMoveToWindow { - if (self.window == nil) { - return; - } - - if ([self.superview isKindOfClass:[RNSTabsBottomAccessoryComponentView class]]) { + if (self.window != nil && [self.superview isKindOfClass:[RNSTabsBottomAccessoryComponentView class]]) { RNSTabsBottomAccessoryComponentView *accessoryView = static_cast(self.superview); _accessoryView = accessoryView; [_accessoryView.helper setContentView:self forEnvironment:_environment]; } else { + // We are leaving the accessory. Detach from the helper so it removes its `hidden` KVO observer while we are still + // alive. Must run on the `window == nil` path too (Fabric unmount removes from superview, then deallocates). [_accessoryView.helper setContentView:nil forEnvironment:_environment]; _accessoryView = nil; } } -// `RCTViewComponentView` uses this deprecated callback to invalidate layer when trait collection -// `hasDifferentColorAppearanceComparedToTraitCollection`. This updates opacity which breaks our -// content view switching workaround. To mitigate this, we update content view visibility after -// RCTViewComponentView handles the change. We need to use the same deprecated callback as it's -// called after callbacks registered via the new API. -- (void)traitCollectionDidChange:(UITraitCollection *)previousTraitCollection -{ - [super traitCollectionDidChange:previousTraitCollection]; - if ([self.traitCollection hasDifferentColorAppearanceComparedToTraitCollection:previousTraitCollection]) { - [_accessoryView.helper handleContentViewVisibilityForEnvironmentIfNeeded]; - } -} - #endif // RNS_TABS_BOTTOM_ACCESSORY_AVAILABLE #pragma mark - RCTViewComponentViewProtocol @@ -75,20 +60,6 @@ - (void)updateProps:(const facebook::react::Props::Shared &)props [super updateProps:props oldProps:oldProps]; } -- (void)finalizeUpdates:(RNComponentViewUpdateMask)updateMask -{ - [super finalizeUpdates:updateMask]; - - // In finalize updates, `invalidateLayer` is called. It resets `view.layer.opacity` - // which we use to switch visible bottom accessory content view. In order to mitigate - // this, we update visibility after `[super finalizeUpdates:updateMask]`. Without this, - // both content views are visible on first render. It does not happen on subsequent - // renders because `updateState` is called before trait changes but there might be other - // cases when `finalizeUpdates` will run so to make sure that we maintain correct - // visibility, we call `handleContentViewVisibilityForEnvironmentIfNeeded` here. - [_accessoryView.helper handleContentViewVisibilityForEnvironmentIfNeeded]; -} - + (react::ComponentDescriptorProvider)componentDescriptorProvider { return react::concreteComponentDescriptorProvider(); diff --git a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.h b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.h index 1af5256f08..a15436c137 100644 --- a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.h +++ b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.h @@ -48,7 +48,7 @@ API_AVAILABLE(ios(26.0)) * thanks to synchronous state updates and work correctly). * In order to mitigate this, we introduced a workaround approach: 2 views are rendered all the time on top of each * other. One is for `regular` environment and the second one is for `inline` environment. When environment changes, we - * swap which view is actually visible by changing opacity. + * swap which view is actually visible by toggling `hidden`. */ @interface RNSTabsBottomAccessoryHelper () @@ -60,7 +60,7 @@ API_AVAILABLE(ios(26.0)) forEnvironment:(RNSTabsBottomAccessoryEnvironment)environment; /** - * If `contentView` is set for both environments, sets opacity according to current tab accessory `environent`. + * If `contentView` is set for both environments, toggles `hidden` according to current tab accessory `environment`. * Otherwise, it is a no-op. */ - (void)handleContentViewVisibilityForEnvironmentIfNeeded; diff --git a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.mm b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.mm index 9095a8bd43..4dc647e3de 100644 --- a/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.mm +++ b/ios/tabs/bottom-accessory/RNSTabsBottomAccessoryHelper.mm @@ -8,13 +8,16 @@ namespace react = facebook::react; static void *RNSTabsBottomAccessoryNativeWrapperViewContext = &RNSTabsBottomAccessoryNativeWrapperViewContext; +static void *RNSTabsBottomAccessoryContentViewHiddenContext = &RNSTabsBottomAccessoryContentViewHiddenContext; @implementation RNSTabsBottomAccessoryHelper { RNSTabsBottomAccessoryComponentView *__weak _bottomAccessoryView; UIView *__weak _observedNativeWrapperView; - RNSTabsBottomAccessoryContentComponentView *__weak _regularContentView; - RNSTabsBottomAccessoryContentComponentView *__weak _inlineContentView; + RNSTabsBottomAccessoryContentComponentView *_regularContentView; + RNSTabsBottomAccessoryContentComponentView *_inlineContentView; + + BOOL _isAdjustingContentViewVisibility; id _traitChangeRegistration; } @@ -35,6 +38,7 @@ - (void)initState _observedNativeWrapperView = nil; _regularContentView = nil; _inlineContentView = nil; + _isAdjustingContentViewVisibility = NO; } #pragma mark - Content view switching workaround @@ -49,11 +53,11 @@ - (void)setContentView:(RNSTabsBottomAccessoryContentComponentView *)contentView { switch (environment) { case RNSTabsBottomAccessoryEnvironmentRegular: - _regularContentView = contentView; + [self replaceContentView:&_regularContentView with:contentView]; break; case RNSTabsBottomAccessoryEnvironmentInline: - _inlineContentView = contentView; + [self replaceContentView:&_inlineContentView with:contentView]; break; default: @@ -69,16 +73,53 @@ - (void)handleContentViewVisibilityForEnvironmentIfNeeded return; } + _isAdjustingContentViewVisibility = YES; + switch (self->_bottomAccessoryView.traitCollection.tabAccessoryEnvironment) { case UITabAccessoryEnvironmentInline: - _regularContentView.layer.opacity = 0.0; - _inlineContentView.layer.opacity = 1.0; + _regularContentView.hidden = YES; + _inlineContentView.hidden = NO; break; default: - _regularContentView.layer.opacity = 1.0; - _inlineContentView.layer.opacity = 0.0; + _regularContentView.hidden = NO; + _inlineContentView.hidden = YES; break; } + + _isAdjustingContentViewVisibility = NO; +} + +#pragma mark - Observing content view hidden changes + +- (void)unregisterForContentViewHiddenChanges:(RNSTabsBottomAccessoryContentComponentView *__strong *)observedView +{ + RNSTabsBottomAccessoryContentComponentView *currentlyObserved = *observedView; + if (currentlyObserved == nil) { + return; + } + + [currentlyObserved removeObserver:self forKeyPath:@"hidden" context:RNSTabsBottomAccessoryContentViewHiddenContext]; + *observedView = nil; +} + +- (void)replaceContentView:(RNSTabsBottomAccessoryContentComponentView *__strong *)slot + with:(nullable RNSTabsBottomAccessoryContentComponentView *)newView +{ + if (*slot == newView) { + return; + } + + // Detach the observer from the previous view first; this also clears the slot. + [self unregisterForContentViewHiddenChanges:slot]; + + if (newView != nil) { + [newView addObserver:self + forKeyPath:@"hidden" + options:NSKeyValueObservingOptionNew + context:RNSTabsBottomAccessoryContentViewHiddenContext]; + } + + *slot = newView; } #pragma mark - Observing environment changes @@ -132,6 +173,10 @@ - (void)observeValueForKeyPath:(NSString *)keyPath { if (context == RNSTabsBottomAccessoryNativeWrapperViewContext) { [self notifyWrapperViewFrameHasChanged]; + } else if (context == RNSTabsBottomAccessoryContentViewHiddenContext) { + if (!_isAdjustingContentViewVisibility) { + [self handleContentViewVisibilityForEnvironmentIfNeeded]; + } } else { [super observeValueForKeyPath:keyPath ofObject:object change:change context:context]; } @@ -158,9 +203,9 @@ - (void)invalidate [_bottomAccessoryView unregisterForTraitChanges:_traitChangeRegistration]; _traitChangeRegistration = nil; [self unregisterForAccessoryFrameChanges]; + [self unregisterForContentViewHiddenChanges:&_regularContentView]; + [self unregisterForContentViewHiddenChanges:&_inlineContentView]; _bottomAccessoryView = nil; - _regularContentView = nil; - _inlineContentView = nil; } @end