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

[WIP] new arch on android #68

Closed

Conversation

sarthak-d11
Copy link
Contributor

No description provided.

@sarthak-d11 sarthak-d11 marked this pull request as draft October 21, 2024 12:33
@sarthak-d11 sarthak-d11 changed the title feat: new arch building on android [WIP] new arch on android Oct 21, 2024
@sarthak-d11 sarthak-d11 mentioned this pull request Oct 21, 2024
@okwasniewski
Copy link
Collaborator

Thanks for working on this!

Just FYI, Android will need a custom C++ Shadow node in order to layout the bottom navigation bar properly.

Here is a reference of this in another library: callstack/react-native-slider#589

We can also split the work (base implementation and Cxx shadow node separately)

@sarthak-d11
Copy link
Contributor Author

sarthak-d11 commented Oct 23, 2024

Hi @okwasniewski
I have added the custom C++ shadow node but I am still facing some issues.
The bottom tab bar is not displayed on the screen but the screens are visible.
Here is a screenshot

Screenshot_1729365312

I have tried layout inspector on this and these are the results

Screenshot 2024-10-24 at 12 55 27 AM Screenshot 2024-10-24 at 1 07 06 AM

If you look at these screenshots we can see that there is some BottomNavigationView is shown in Layout Inspector. But it is not displayed on the screen. In case of React native slider the issue was with height and width but here we don't have a similiar issue.
Please let me know your thoughts about this

@okwasniewski
Copy link
Collaborator

Hey, I merged #64, I am looking into this!

@okwasniewski
Copy link
Collaborator

Hey,

Huge thanks for working on this! I've started off your work and fixed all the issues here: #79

@sarthak-d11
Copy link
Contributor Author

Hi @okwasniewski, I appreciate your review of this PR; could we discuss the mistake I made regarding the Shadow Node (Was it Event Dispatcher or anything else), as I'm unsure about it, and could you also recommend any resources for further reading(Specially for Custom Shadow Node and CPP code of RN)?

@okwasniewski
Copy link
Collaborator

okwasniewski commented Oct 28, 2024

Hi @okwasniewski, I appreciate your review of this PR; could we discuss the mistake I made regarding the Shadow Node (Was it Event Dispatcher or anything else), as I'm unsure about it, and could you also recommend any resources for further reading(Specially for Custom Shadow Node and CPP code of RN)?

Sure! Sorry for not providing you any feedback.

I've spent a lot of time figuring out whats was wrong you were mostly on point but the main issue was the method signature of the measure call on the ViewManager level as it wasn't called at all. I also made it work for iOS because when you opt-out of codegen passing interfaceOnly it's not generated for iOS either.

Regarding the dispatcher I removed it and attached it separately for both architectures as I think this makes things simpler, otherwise it was great work!

Regarding resources, I don't think there are any... I was mostly going through RN source code and checking how it's done for built-in components

@okwasniewski
Copy link
Collaborator

I've merged #79 once again thanks for your help!

@sarthak-d11
Copy link
Contributor Author

sarthak-d11 commented Nov 6, 2024

@okwasniewski Thanks for your feedback this is really helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants