callstack / react-native-slider

React Native component exposing Slider from iOS and SeekBar from Android
MIT License
1.22k stars 270 forks source link

slider has 0 height on Android with new architecture enabled #652

Closed vonovak closed 1 month ago

vonovak commented 1 month ago

Description

[android only] While testing the project (I tested from the main branch) with new architecture (RN 0.75) I experienced 2 issues:

  1. project does not build following https://github.com/callstack/react-native-slider/pull/649 because the codegen-generated interface expects floats instead of doubles - that's hopefully easy to fix
  2. slider cannot be interacted with (dragged) because it has zero height; see image
Screenshot 2024-10-05 at 0 00 02

This seems like a regression since https://github.com/callstack/react-native-slider/pull/589 which was solving the same problem. I'd be happy to submit a PR instead of an issue but I believe you'll be much better at figuring it out :).

Reproducible Demo

Please see the example at https://github.com/vonovak/react-native-slider/tree/fix/new-arch-issues/new-example

It's a fork of this repo, with new example added which uses https://github.com/microsoft/react-native-test-app (RNTA)

The example app was generated with npx --package react-native-test-app@latest init and then I modified metro.config.js and react-native.config.js to make it all work. Then I copied src directory from the existing example.

Feel free to integrate that into the repo, RNTA is an amazing tool :).

draggie commented 1 month ago

Hi @vonovak, thank you for reporting the issue. I have been able to solve the case with the codegen functionality and the fix is ready.

However regarding the second issue - I have created new project with RN 75 and new arch enabled and the problem you have described does not exist, although I also was able to reproduce this while using the RNTA example.

Since in basic usage this seems to be not relevant, I will not provide any fix for that, maybe this is some issue coming from RNTA itself. However from what I have seen the package can be patched by simply defining something like: const sliderStyle = {zIndex: 1, width: width, height: 10}; and using tool like patch-package to solve problem for this particular scenario.

okwasniewski commented 1 month ago

Hey!

Thanks for reporting the issue. I'm going to tag @tido64 here as this might be an issue inside of RNTA. We observed that the bare RN App calls the shadow node measure() function but RNTA doesn't.

Any idea why this might happen?

vonovak commented 1 month ago

Hi! Not sure why that would happen, but I saw the same behavior in https://github.com/expo/expo/blob/main/apps/bare-expo/package.json which is a more complete application with a bunch of dependencies, so I expect this to be an issue also outside of RNTA. Unfortunately, manually providing height didn't work for me.