refact: merge Fabric & Paper impls: ScreenStackHeaderConfig (3) #1417
Conversation
RNSScreenController to RNSScreen
It caused RN_FABRIC_ENABLED flag to be invalid (as it merged with
previous flag from the list...)
Otherwise C++ standard headers are not visible from .m files -> project fails to build. e.g. Whem=n A.m includes B.h & B.h imports memory header: "#include <memory>" -- memory header is not visible & compilation fails
paper specific section
1. move the prop to implemented section in JS 2. move the prop to "common" section in RNSScreenView interface
specific section
specific section Aim: to ake project compile
most likely some kind of merging artifact
most likely some kind of merge artifact
temporary solution until customAnimationOnSwipe is not added to RNSScreenView on Fabric
these directives should be removed once implementations ale fully merged
to Paper specific section This method is not present in RNSScreenStackComponentView, therefore I conclude it is not necessary at least for now
Fabric specific section
…c-to-paper-screen-stack
…r/merge-fabric-to-paper-screen-stack-header-config
| RNSScreenStackHeaderConfigComponentView *config = | ||
| (RNSScreenStackHeaderConfigComponentView *)((RNSScreenView *)view).config; | ||
| [RNSScreenStackHeaderConfigComponentView willShowViewController:viewController animated:animated withConfig:config]; | ||
| RNSScreenStackHeaderConfig *config = (RNSScreenStackHeaderConfig *)((RNSScreenView *)view).config; |
There was a problem hiding this comment.
Could you debug why this code is added and the previous implementation is not used instead?
There was a problem hiding this comment.
I do not understand this completely, but paper implementation of RNSScreenView does not expose config property. In Fabric implementation this prop is updated when child component view is being mounted (mountChildComponentView method) or unmounted (unmountChildComponentView method)
There was a problem hiding this comment.
Hmm but we could you old implementation on Fabric too, it is possible to traverse react children of the view there too I think. The question is was it breaking something?
| for (RNSScreenStackHeaderSubviewComponentView *subview in config.reactSubviews) { | ||
| switch (subview.type) { | ||
| case facebook::react::RNSScreenStackHeaderSubviewType::Left: { | ||
| //#if !TARGET_OS_TV |
There was a problem hiding this comment.
@property (nonatomic) BOOL backButtonInCustomView; is not implemented yet on Fabric.
I do not resolve this conversation as it requires to be added in following PR
This is required as RCTComponentViewProtocol is not visible on paper
layoutNavigationControllerView
| RNSScreenStackHeaderConfigComponentView *config = | ||
| (RNSScreenStackHeaderConfigComponentView *)((RNSScreenView *)view).config; | ||
| [RNSScreenStackHeaderConfigComponentView willShowViewController:viewController animated:animated withConfig:config]; | ||
| RNSScreenStackHeaderConfig *config = (RNSScreenStackHeaderConfig *)((RNSScreenView *)view).config; |
There was a problem hiding this comment.
Hmm but we could you old implementation on Fabric too, it is possible to traverse react children of the view there too I think. The question is was it breaking something?
merge artifacts!
also remove redeclaration in implementation
This is changed when ScreenStackHeaderSubview is merged
kkafar
deleted the
@kkafar/merge-fabric-to-paper-screen-stack-header-config
branch
Description
PRs mentioned just above aim to merge new Fabric components implementation to Paper.
They should be merged in ascending order (each PR has a order number associated).
Test code and steps to reproduce
Checklist