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

Edge-to-edge support #103

Merged
merged 7 commits into from
Nov 12, 2024
Merged

Edge-to-edge support #103

merged 7 commits into from
Nov 12, 2024

Conversation

zoontek
Copy link
Contributor

@zoontek zoontek commented Nov 10, 2024

@lodev09 Hi 👋

Following #72

I had to use decorView.systemUiVisibility = View.SYSTEM_UI_FLAG_LAYOUT_STABLE or View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION to ignore insets applied on the BottomSheetDialog internal bottomSheet FrameLayout (related issue). The alternative would be to fork and remove all the things we don't want, but…meh 😄.

Tested on Android 6 to 15.

It's automatic when react-native-edge-to-edge is installed, so all you need to test it is to install the package, and the expo plugin and run yarn expo prebuild --clean && yarn android.

react-native-true-sheet.mp4

Copy link

vercel bot commented Nov 10, 2024

@zoontek is attempting to deploy a commit to the Jovanni's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

codeclimate bot commented Nov 10, 2024

Code Climate has analyzed commit ad825ac and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@lodev09
Copy link
Owner

lodev09 commented Nov 11, 2024

Hey @zoontek, thank you so much for this!

If you don't mind, can we also add EDGE_TO_EDGE as a prop in the component as well?

@zoontek
Copy link
Contributor Author

zoontek commented Nov 11, 2024

@lodev09 , Sure, here it is. The prop value cannot be updated once the dialog is presented or we would have to add a mechanism similar to RN Modal (which dismiss and recreate the Dialog). I think it's acceptable, as your app is edge-to-edge or not and that's not something that change at runtime.

react-native-is-edge-to-edge has been added (it's super lightweight) for edgeToEdge prop default, similar to other libraries using it.

@lodev09
Copy link
Owner

lodev09 commented Nov 11, 2024

I think we can just put the burden to them on installing/detecting it 😂. So a simple prop would suffice. What do you think?

We can install both your lib in the example though, for demonstration purposes. I will update the docs in a separate PR.

Thanks again for this @zoontek

@zoontek
Copy link
Contributor Author

zoontek commented Nov 11, 2024

I think we can just put the burden to them on installing/detecting it 😂. So a simple prop would suffice. What do you think?

I disagree.

react-native-is-edge-to-edge is not react-native-edge-to-edge, it's a simple library without native code (250 bytes, not even gzipped - and other libraries are also using it), just used for detection and to avoid a configuration nightmare between all the libraries that somehow support edge-to-edge.

Currently, a lot of react-native app are not edge-to-edge because it doesn't "just work". Having good defaults can save a lot of developers time.

@lodev09
Copy link
Owner

lodev09 commented Nov 12, 2024

Okay, you do have a point. However, I'm one of those guys that tries to avoid unnecessary dependencies as much as possible 😅

Maybe you can revert it back to checking in the native side instead? If I'm not mistaken, this util checks if react-native-edge-to-edge is installed right?

So we check internally if react-native-edge-to-edge is installed AND also give them option to pass it as a prop.

What do you think?

@zoontek
Copy link
Contributor Author

zoontek commented Nov 12, 2024

@lodev09 react-native-is-edge-to-edge has another advantage compared to Kotlin check: if we continue and merge this proposal, the package will be updated to detect both: first, if edge-to-edge is enabled in RN core, then if it isn't (let's say the user hasn't updated yet), if the user has installed react-native-edge-to-edge.

You will have nothing to do on your side, the transition will be smooth. People will be able to set enableEdgeToEdge=true and uninstall react-native-edge-to-edge without thinking about react-native-true-sheet.

Otherwise, you will need to perform a new release to handle both in Kotlin.

@lodev09
Copy link
Owner

lodev09 commented Nov 12, 2024

Looks good @zoontek. Thanks for your contribution 💪

@lodev09 lodev09 merged commit 5a0865c into lodev09:main Nov 12, 2024
3 of 6 checks passed
@zoontek zoontek deleted the edge-to-edge-support branch November 12, 2024 20:31
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