From 16e52f68c3b66eee8922c6182ccf13911cdb2f43 Mon Sep 17 00:00:00 2001 From: yogevbd Date: Thu, 5 Sep 2019 15:00:12 +0300 Subject: [PATCH 1/2] Fix button with react component doesn't get unmounted --- e2e/Options.test.js | 6 +++++ lib/ios/RNNNavigationButtons.m | 2 +- lib/ios/RNNReactComponentRegistry.h | 2 ++ lib/ios/RNNReactComponentRegistry.m | 10 +++++++ lib/ios/RNNReactView.h | 2 ++ lib/ios/RNNReactView.m | 6 ++++- lib/ios/RNNUIBarButtonItem.h | 3 ++- lib/ios/RNNUIBarButtonItem.m | 11 +++++++- playground/src/screens/LifecycleButton.js | 9 +++++++ playground/src/screens/OptionsScreen.js | 32 +++++++++++++++++++++-- playground/src/screens/Screens.js | 1 + playground/src/screens/index.js | 3 +++ playground/src/testIDs.js | 2 ++ 13 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 playground/src/screens/LifecycleButton.js diff --git a/e2e/Options.test.js b/e2e/Options.test.js index 5a7fe5fe981..9179705141b 100644 --- a/e2e/Options.test.js +++ b/e2e/Options.test.js @@ -88,6 +88,12 @@ describe('Options', () => { await expect(elementByLabel('Styling Options')).toBeVisible(); }); + it('Reseting buttons should unmount button react view', async () => { + await elementById(TestIDs.SHOW_LIFECYCLE_BTN).tap(); + await elementById(TestIDs.RESET_BUTTONS).tap(); + await expect(elementByLabel('Button component unmounted')).toBeVisible(); + }); + xit('hides topBar onScroll down and shows it on scroll up', async () => { await elementById(TestIDs.PUSH_OPTIONS_BUTTON).tap(); await elementById(TestIDs.SCROLLVIEW_SCREEN_BUTTON).tap(); diff --git a/lib/ios/RNNNavigationButtons.m b/lib/ios/RNNNavigationButtons.m index 1bb41e06ea7..079e3797202 100644 --- a/lib/ios/RNNNavigationButtons.m +++ b/lib/ios/RNNNavigationButtons.m @@ -103,7 +103,7 @@ -(RNNUIBarButtonItem*)buildButton: (NSDictionary*)dictionary defaultStyle:(RNNBu componentOptions.name = [[Text alloc] initWithValue:component[@"name"]]; RNNReactView *view = [_componentRegistry createComponentIfNotExists:componentOptions parentComponentId:self.viewController.layoutInfo.componentId reactViewReadyBlock:nil]; - barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withCustomView:view]; + barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withCustomView:view componentRegistry:_componentRegistry]; } else if (iconImage) { barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withIcon:iconImage]; } else if (title) { diff --git a/lib/ios/RNNReactComponentRegistry.h b/lib/ios/RNNReactComponentRegistry.h index ec3ff89fdbb..1c9fe513db1 100644 --- a/lib/ios/RNNReactComponentRegistry.h +++ b/lib/ios/RNNReactComponentRegistry.h @@ -11,6 +11,8 @@ - (void)removeComponent:(NSString *)componentId; +- (void)removeChildComponent:(NSString *)childId; + - (void)clearComponentsForParentId:(NSString *)parentComponentId; - (void)clear; diff --git a/lib/ios/RNNReactComponentRegistry.m b/lib/ios/RNNReactComponentRegistry.m index 09e9082521c..8de50647b35 100644 --- a/lib/ios/RNNReactComponentRegistry.m +++ b/lib/ios/RNNReactComponentRegistry.m @@ -48,6 +48,16 @@ - (void)removeComponent:(NSString *)componentId { } } +- (void)removeChildComponent:(NSString *)childId { + NSMutableDictionary* parent; + while ((parent = _componentStore.objectEnumerator.nextObject)) { + if ([parent objectForKey:childId]) { + [parent removeObjectForKey:childId]; + return; + } + } +} + - (void)clear { [_componentStore removeAllObjects]; } diff --git a/lib/ios/RNNReactView.h b/lib/ios/RNNReactView.h index fbd8b35b18f..a435b2b2ef1 100644 --- a/lib/ios/RNNReactView.h +++ b/lib/ios/RNNReactView.h @@ -12,4 +12,6 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void); - (void)setAlignment:(NSString *)alignment inFrame:(CGRect)frame; +- (NSString *)componentId; + @end diff --git a/lib/ios/RNNReactView.m b/lib/ios/RNNReactView.m index a6ed5802057..b45369a7e80 100644 --- a/lib/ios/RNNReactView.m +++ b/lib/ios/RNNReactView.m @@ -22,13 +22,17 @@ - (void)contentDidAppear:(NSNotification *)notification { RNNReactView* appearedView = notification.object; - if (_reactViewReadyBlock && [appearedView.appProperties[@"componentId"] isEqual:self.appProperties[@"componentId"]]) { + if (_reactViewReadyBlock && [appearedView.appProperties[@"componentId"] isEqual:self.componentId]) { _reactViewReadyBlock(); _reactViewReadyBlock = nil; [[NSNotificationCenter defaultCenter] removeObserver:self]; } } +- (NSString *)componentId { + return self.appProperties[@"componentId"]; +} + - (void)setRootViewDidChangeIntrinsicSize:(void (^)(CGSize))rootViewDidChangeIntrinsicSize { _rootViewDidChangeIntrinsicSize = rootViewDidChangeIntrinsicSize; self.delegate = self; diff --git a/lib/ios/RNNUIBarButtonItem.h b/lib/ios/RNNUIBarButtonItem.h index 40269a0aabc..9a4db8d7adf 100644 --- a/lib/ios/RNNUIBarButtonItem.h +++ b/lib/ios/RNNUIBarButtonItem.h @@ -1,6 +1,7 @@ #import #import #import +#import "RNNReactComponentRegistry.h" @interface RNNUIBarButtonItem : UIBarButtonItem @@ -8,7 +9,7 @@ -(instancetype)init:(NSString*)buttonId withIcon:(UIImage*)iconImage; -(instancetype)init:(NSString*)buttonId withTitle:(NSString*)title; --(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView*)reactView; +-(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView componentRegistry:(RNNReactComponentRegistry *)componentRegistry; -(instancetype)init:(NSString*)buttonId withSystemItem:(NSString*)systemItemName; @end diff --git a/lib/ios/RNNUIBarButtonItem.m b/lib/ios/RNNUIBarButtonItem.m index d7bb6ffe79b..32b78108923 100644 --- a/lib/ios/RNNUIBarButtonItem.m +++ b/lib/ios/RNNUIBarButtonItem.m @@ -7,6 +7,7 @@ @interface RNNUIBarButtonItem () @property (nonatomic, strong) NSLayoutConstraint *widthConstraint; @property (nonatomic, strong) NSLayoutConstraint *heightConstraint; +@property (nonatomic, weak) RNNReactComponentRegistry *componentRegistry; @end @@ -28,9 +29,10 @@ -(instancetype)init:(NSString*)buttonId withTitle:(NSString*)title { return self; } --(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView { +-(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView componentRegistry:(RNNReactComponentRegistry *)componentRegistry { self = [super initWithCustomView:reactView]; + self.componentRegistry = componentRegistry; reactView.sizeFlexibility = RCTRootViewSizeFlexibilityWidthAndHeight; reactView.delegate = self; reactView.backgroundColor = [UIColor clearColor]; @@ -76,4 +78,11 @@ - (void)onButtonPressed { afterDelay:0]; } +- (void)dealloc { + if ([self.customView isKindOfClass:[RNNReactView class]]) { + RNNReactView* customView = self.customView; + [self.componentRegistry removeChildComponent:customView.componentId]; + } +} + @end diff --git a/playground/src/screens/LifecycleButton.js b/playground/src/screens/LifecycleButton.js new file mode 100644 index 00000000000..a813ca22e6d --- /dev/null +++ b/playground/src/screens/LifecycleButton.js @@ -0,0 +1,9 @@ +const RoundedButton = require('./RoundedButton'); + +class LifecycleButton extends RoundedButton { + componentWillUnmount() { + alert('Button component unmounted'); + } +} + +module.exports = LifecycleButton; diff --git a/playground/src/screens/OptionsScreen.js b/playground/src/screens/OptionsScreen.js index cdedec8c2fb..63f54dc0fcb 100644 --- a/playground/src/screens/OptionsScreen.js +++ b/playground/src/screens/OptionsScreen.js @@ -1,5 +1,5 @@ const React = require('react'); -const { Component } = require('react'); +const {Component} = require('react'); const Root = require('../components/Root'); const Button = require('../components/Button') const Navigation = require('../services/Navigation'); @@ -16,7 +16,9 @@ const { PUSH_BTN, HIDE_TOPBAR_DEFAULT_OPTIONS, SHOW_YELLOW_BOX_BTN, - SET_REACT_TITLE_VIEW + SET_REACT_TITLE_VIEW, + RESET_BUTTONS, + SHOW_LIFECYCLE_BTN } = require('../testIDs'); class Options extends Component { @@ -69,10 +71,36 @@ class Options extends Component {