-
-
Notifications
You must be signed in to change notification settings - Fork 514
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
chore: unify business logic between archs (8) #1446
chore: unify business logic between archs (8) #1446
Conversation
ios/RNSScreen.mm
Outdated
@@ -783,12 +778,16 @@ - (void)viewDidLayoutSubviews | |||
BOOL isDisplayedWithinUINavController = | |||
[self.parentViewController isKindOfClass:[RNScreensNavigationController class]]; | |||
BOOL isPresentedAsNativeModal = self.parentViewController == nil && self.presentingViewController != nil; | |||
|
|||
if ((isDisplayedWithinUINavController || isPresentedAsNativeModal) && | |||
!CGRectEqualToRect(_lastViewFrame, self.view.frame)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition: !CGRectEqualToRect(_lastViewFrame, self.view.frame)
is not present in Fabric and it might destroy the behavior so it would be better to check it twice since there was a reason why it was not added there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See: e6d1655
ec4e7d1
to
3524fbc
Compare
63e4938
to
e6d1655
Compare
3524fbc
to
a40a91a
Compare
e6d1655
to
bf1a835
Compare
helper methods shared #1416 (comment)
RNSScreenStackHeaderConfig.h
RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig
bf1a835
to
8a9e59d
Compare
It changed after I rebased
* chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
…6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
* chore: move gestureResponseDistance prop to shared section in RNSScreen interface * refact: linter suggestions * chore: add gestureResponseDistanceProp to Codegen * chore: add conversion methods for GestureResponseDistanceStruct * refact: linter suggestion * chore: add props update for Fabric As there is no way for creating custom `equality operator` for custom struct via codegen && such check (newProps != oldProps) would require three "ands" I think it is easier and potentially cheaper to just assign this (w/o surrounding if-check) * chore: unify RNSScreenStack#gestureRecognizerShouldBegin method * chore: move RNSScreenStackView#isInGestureResponseDistance:gestureRecognizer: method to common section This is now possible, as `gestureResponseDistance` prop is available on Fabric * chore: add noop for Android This prop is annotated as iOS only in docs * chore: change GestureResponseDistance field type from int to float * fix: merge artifact * chore: add deafult value for gestureResponseDistance on JS side It is still to be checked whether it is only place where this should be added * chore: unify RNSScreenStack#isInGestureResponseDistance:topScreen: method * chore: add `homeIndicatorHidden` & `activityState` props for Fabric (6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
* chore: add customAnimationOnSwipe prop to codegen * chore: move customAnimationOnSwipe prop to shared section in RNSScreen interface * refact: linter suggestion * chore: add customAnimationOnSwipe update for Fabric * chore: unify RNSScreenStackAnimator#animateTransition method This is possible as customAnimationOnSwipe prop is now implemented on Fabric * chore: unify RNSScreenStack#gestureRecognizer should begin method This is possible as customAnimationOnSwipe prop in now implemented on Fabric * chore: noop for Android This prop is annotated as iOS only in docs * chore: remove not needed difference between implementations * chore: add `gestureResponseDistance` prop for Fabric (5) (#1442) * chore: move gestureResponseDistance prop to shared section in RNSScreen interface * refact: linter suggestions * chore: add gestureResponseDistanceProp to Codegen * chore: add conversion methods for GestureResponseDistanceStruct * refact: linter suggestion * chore: add props update for Fabric As there is no way for creating custom `equality operator` for custom struct via codegen && such check (newProps != oldProps) would require three "ands" I think it is easier and potentially cheaper to just assign this (w/o surrounding if-check) * chore: unify RNSScreenStack#gestureRecognizerShouldBegin method * chore: move RNSScreenStackView#isInGestureResponseDistance:gestureRecognizer: method to common section This is now possible, as `gestureResponseDistance` prop is available on Fabric * chore: add noop for Android This prop is annotated as iOS only in docs * chore: change GestureResponseDistance field type from int to float * fix: merge artifact * chore: add deafult value for gestureResponseDistance on JS side It is still to be checked whether it is only place where this should be added * chore: unify RNSScreenStack#isInGestureResponseDistance:topScreen: method * chore: add `homeIndicatorHidden` & `activityState` props for Fabric (6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
* chore: move hideKeyboardOnSwipe prop to common section in RNSScreen header * chore: add hideKeyboardOnSwipe prop to codegen * chore: make keyboard dismiss logic common * chore: add hideKeyboardOnSwipe prop update for Fabric * chore: add noop for Android This prop is marked as iOS only * chore: add `customAnimationOnSwipe` prop for Fabric (4) (#1434) * chore: add customAnimationOnSwipe prop to codegen * chore: move customAnimationOnSwipe prop to shared section in RNSScreen interface * refact: linter suggestion * chore: add customAnimationOnSwipe update for Fabric * chore: unify RNSScreenStackAnimator#animateTransition method This is possible as customAnimationOnSwipe prop is now implemented on Fabric * chore: unify RNSScreenStack#gestureRecognizer should begin method This is possible as customAnimationOnSwipe prop in now implemented on Fabric * chore: noop for Android This prop is annotated as iOS only in docs * chore: remove not needed difference between implementations * chore: add `gestureResponseDistance` prop for Fabric (5) (#1442) * chore: move gestureResponseDistance prop to shared section in RNSScreen interface * refact: linter suggestions * chore: add gestureResponseDistanceProp to Codegen * chore: add conversion methods for GestureResponseDistanceStruct * refact: linter suggestion * chore: add props update for Fabric As there is no way for creating custom `equality operator` for custom struct via codegen && such check (newProps != oldProps) would require three "ands" I think it is easier and potentially cheaper to just assign this (w/o surrounding if-check) * chore: unify RNSScreenStack#gestureRecognizerShouldBegin method * chore: move RNSScreenStackView#isInGestureResponseDistance:gestureRecognizer: method to common section This is now possible, as `gestureResponseDistance` prop is available on Fabric * chore: add noop for Android This prop is annotated as iOS only in docs * chore: change GestureResponseDistance field type from int to float * fix: merge artifact * chore: add deafult value for gestureResponseDistance on JS side It is still to be checked whether it is only place where this should be added * chore: unify RNSScreenStack#isInGestureResponseDistance:topScreen: method * chore: add `homeIndicatorHidden` & `activityState` props for Fabric (6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
* chore: add method for converting RNSScreenReplaceAnimation enum from Cpp to obj-c * chore: update replaceAnimation prop on Fabric * refact: linter suggestion * chore: add `hideKeyboardOnSwipe` prop for Fabric (3) (#1433) * chore: move hideKeyboardOnSwipe prop to common section in RNSScreen header * chore: add hideKeyboardOnSwipe prop to codegen * chore: make keyboard dismiss logic common * chore: add hideKeyboardOnSwipe prop update for Fabric * chore: add noop for Android This prop is marked as iOS only * chore: add `customAnimationOnSwipe` prop for Fabric (4) (#1434) * chore: add customAnimationOnSwipe prop to codegen * chore: move customAnimationOnSwipe prop to shared section in RNSScreen interface * refact: linter suggestion * chore: add customAnimationOnSwipe update for Fabric * chore: unify RNSScreenStackAnimator#animateTransition method This is possible as customAnimationOnSwipe prop is now implemented on Fabric * chore: unify RNSScreenStack#gestureRecognizer should begin method This is possible as customAnimationOnSwipe prop in now implemented on Fabric * chore: noop for Android This prop is annotated as iOS only in docs * chore: remove not needed difference between implementations * chore: add `gestureResponseDistance` prop for Fabric (5) (#1442) * chore: move gestureResponseDistance prop to shared section in RNSScreen interface * refact: linter suggestions * chore: add gestureResponseDistanceProp to Codegen * chore: add conversion methods for GestureResponseDistanceStruct * refact: linter suggestion * chore: add props update for Fabric As there is no way for creating custom `equality operator` for custom struct via codegen && such check (newProps != oldProps) would require three "ands" I think it is easier and potentially cheaper to just assign this (w/o surrounding if-check) * chore: unify RNSScreenStack#gestureRecognizerShouldBegin method * chore: move RNSScreenStackView#isInGestureResponseDistance:gestureRecognizer: method to common section This is now possible, as `gestureResponseDistance` prop is available on Fabric * chore: add noop for Android This prop is annotated as iOS only in docs * chore: change GestureResponseDistance field type from int to float * fix: merge artifact * chore: add deafult value for gestureResponseDistance on JS side It is still to be checked whether it is only place where this should be added * chore: unify RNSScreenStack#isInGestureResponseDistance:topScreen: method * chore: add `homeIndicatorHidden` & `activityState` props for Fabric (6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
* chore: unify RNSScreenStack#updateContainer method This is now possible as dismissed prop is implemented on both platforms * fix: set _dismissed = NO in prepareForRecycle As the component gets recycled, we must re-initialize the prop * chore: add `replaceAnimation` prop for Fabric (2) (#1432) * chore: add method for converting RNSScreenReplaceAnimation enum from Cpp to obj-c * chore: update replaceAnimation prop on Fabric * refact: linter suggestion * chore: add `hideKeyboardOnSwipe` prop for Fabric (3) (#1433) * chore: move hideKeyboardOnSwipe prop to common section in RNSScreen header * chore: add hideKeyboardOnSwipe prop to codegen * chore: make keyboard dismiss logic common * chore: add hideKeyboardOnSwipe prop update for Fabric * chore: add noop for Android This prop is marked as iOS only * chore: add `customAnimationOnSwipe` prop for Fabric (4) (#1434) * chore: add customAnimationOnSwipe prop to codegen * chore: move customAnimationOnSwipe prop to shared section in RNSScreen interface * refact: linter suggestion * chore: add customAnimationOnSwipe update for Fabric * chore: unify RNSScreenStackAnimator#animateTransition method This is possible as customAnimationOnSwipe prop is now implemented on Fabric * chore: unify RNSScreenStack#gestureRecognizer should begin method This is possible as customAnimationOnSwipe prop in now implemented on Fabric * chore: noop for Android This prop is annotated as iOS only in docs * chore: remove not needed difference between implementations * chore: add `gestureResponseDistance` prop for Fabric (5) (#1442) * chore: move gestureResponseDistance prop to shared section in RNSScreen interface * refact: linter suggestions * chore: add gestureResponseDistanceProp to Codegen * chore: add conversion methods for GestureResponseDistanceStruct * refact: linter suggestion * chore: add props update for Fabric As there is no way for creating custom `equality operator` for custom struct via codegen && such check (newProps != oldProps) would require three "ands" I think it is easier and potentially cheaper to just assign this (w/o surrounding if-check) * chore: unify RNSScreenStack#gestureRecognizerShouldBegin method * chore: move RNSScreenStackView#isInGestureResponseDistance:gestureRecognizer: method to common section This is now possible, as `gestureResponseDistance` prop is available on Fabric * chore: add noop for Android This prop is annotated as iOS only in docs * chore: change GestureResponseDistance field type from int to float * fix: merge artifact * chore: add deafult value for gestureResponseDistance on JS side It is still to be checked whether it is only place where this should be added * chore: unify RNSScreenStack#isInGestureResponseDistance:topScreen: method * chore: add `homeIndicatorHidden` & `activityState` props for Fabric (6) (#1443) * chore: add homeIndicatorHidden prop to codegen * chore: add prop update on Fabric * chore: add noop for Android This prop is iOS only * chore: move `activityState` prop to common section in RNSScreen interface * chore: move activityState setter to common section in RNSScreenView impl * refact: linter suggestion * chore: make RNSScreenView#reactSuperview common & make it implement RNSScreenContainerDelegate protocol This hasn't been tested yet. However in any instance it crashes (as the protocol might be not implemented) we can just implement noops for Fabric * chore: move activityState to implemented section in codegen config files * chore: make series of methods shared (see details) * `RNSScreenView#notifyFinishTransitioning` * `RNSScreen#notifyFinishTransitioning` * `willMoveToParentViewController` * `RNSScreenContainer#updateContainer` These methods were coupled together. * fix: handle activityState on Fabric Default value for the prop (when it is not set) on Fabric is -1 instead of nil (I believe we can not get nil from C++) thus this improved condition should work for both archs. * chore: add `preventNativeDismiss` prop for Fabric (7) (#1444) * chore: move preventNativeDismiss prop to common section in RNSScreen interface * fix: merge artifact * chore: add preventNativeDismiss prop to codegen * chore: add preventNativeDismiss prop update * fix: add noop for Android * chore: unify business logic between archs (8) (#1446) * chore: unify RNSScreen#vieDidLayoutSubviews * chore: make RNSScreenStack#hitTest:withEvent: method shared #1416 (comment) * chore: make RNSScreenStackView#isScrollViewPanGestureRecongnizer & some helper methods shared #1416 (comment) * chore: move backButtonInCustomView to shared sectionin RNSScreenStackHeaderConfig.h * chore: move backButtonInCustomView prop to implemented section in JS * chore: update `backButtonInCustomView` for Fabric * chore: unify RNSScreenStackHeaderConfigView#willShowViewController:animated:withConfig * refact: linter suggestion * fix: RNSScreen viewDidLayoutSubviews * chore: copy interfaces to android/src/paper directory * fix: commit address in comment It changed after I rebased
Description
Part of stack PR. See:
This PR unifies some business logic between Paper & Fabric implementations.
Changes
See commits.
Test code and steps to reproduce
TODO
Checklist