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

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

Closed
vonovak opened this issue Oct 4, 2024 · 3 comments · Fixed by #654
Closed

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

vonovak opened this issue Oct 4, 2024 · 3 comments · Fixed by #654
Assignees
Labels
bug report Something isn't working platform: Android Issue related to Android platform

Comments

@vonovak
Copy link
Contributor

vonovak commented Oct 4, 2024

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 fix: floating number problem on android #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 #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 :).

@vonovak vonovak added the bug report Something isn't working label Oct 4, 2024
@draggie draggie self-assigned this Oct 7, 2024
@draggie
Copy link
Contributor

draggie commented Oct 8, 2024

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
Copy link
Member

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
Copy link
Contributor Author

vonovak commented Oct 8, 2024

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.

@github-project-automation github-project-automation bot moved this to To be analyzed in Slider-Board Oct 8, 2024
@BartoszKlonowski BartoszKlonowski moved this from To be analyzed to In Progress in Slider-Board Oct 8, 2024
@draggie draggie added the platform: Android Issue related to Android platform label Oct 10, 2024
chrfalch added a commit to chrfalch/react-native-slider that referenced this issue Oct 10, 2024
When resolving the version inside `react-native-config` which is used by the autolinking gradle tasks, the wrong use of require was used.

This fix changes from using `require.main.require` -> `require`.

The difference is that require.main.require resolves from the entry point (main script), while require resolves from the location of the current file.

Closes callstack#652
@github-project-automation github-project-automation bot moved this from In Progress to Done in Slider-Board Oct 14, 2024
vonovak added a commit to expo/expo that referenced this issue Oct 27, 2024
# Why

to resolve 

callstack/react-native-slider#652
callstack/react-native-slider#660

# How

updated the package

# Test Plan

slider works in bare expo. iOS is smooth, Android is somewhat "jumpy"
which I'll report to the package maintainers

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Something isn't working platform: Android Issue related to Android platform
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants