search

PedroBern / react-native-collapsible-tab-view

A cross-platform Collapsible Tab View component for React Native
MIT License
830 stars 161 forks source link

feat: add `width` prop to `Tabs.Container` #208

Closed filipengberg closed 2 years ago

filipengberg commented 2 years ago

First of all, thank you for a really great library! It deals with some really hard to pull off user interactions!

In an app we're building we get an issue where the window width is miscalulated after having rotated from portrait to landscape and then back to portrait. React native's built in useWindowDimensions hook will still return the landscape width even if the app is rotated back to portrait mode.

Our specific case is probably quite extraordinary, we actually switch between a portrait UIWindow to a landscape UIWindow and then back. And when we do we still get landscape width which causes parts of the tab view to render outside of the screen. When swiping between tabs the app will crash.

Replacing the useWindowDimensions hook with useSafeAreaFrame from react-native-safe-area-context solvess this problem entirely.

We previously used useWindowDimensions in other places in our app but replaced them with useSafeAreaFrame and it fixed several issues we've had with wrong or not updating values. Looking through react native github issues it seems other people have similar problems with this hook: #29290 #31692 #30371 #32279

I understand that this change introduces a new peer dependency on the project but I really think it's worth it in this case!

andreialecu commented 2 years ago

Interesting. Thanks for looking into this.

This would be a breaking change though.

One way around it, which would still allow you to achieve what you want would be to allow passing the width as an optional prop while still defaulting it internally to what useWindowDimensions returns.

That way you'd be able to pass your own width which you can get from useSafeAreaFrame. Additionally, it will also allow the library to work properly if someone ever needs to work with a width smaller than the screen width (maybe on iPad, web, etc).

If you could look into this alternate implementation, then I think that'd be something we'd prefer merging.

filipengberg commented 2 years ago

Thats a great idea! I have updated the PR to add this functionality instead. Not sure if you want to add an example for this? It is kind of an unusual usecase I guess.

Also seem to be getting some type errors about @ts-expect-error. Not sure if I should remove those?

andreialecu commented 2 years ago

Thank you, released as 4.4.0 🚀