expo / react-native-action-sheet

A cross-platform ActionSheet for React Native
MIT License
1.4k stars 226 forks source link

Docs should be more clear #201

Closed yybot1 closed 3 years ago

yybot1 commented 3 years ago
  1. When using useActionSheet hook, connectActionSheet is not needed. This should be more clear in the docs.

  2. When using this library with a navigation library, just wrap top-level component with ActionSheetProvider may not work. For wix/react-native-navigation, each component need to be wrapped separately inside Navigation.registerComponent. I know this is not a react-native-action-sheet problem, it's better if you can add a notation.

Thanks for your work on react-native-action-sheet

bradbyte commented 3 years ago

Hi, I don't believe the first point is correct, the hook taps into the context API and needs the provider to wrap the app so that the correct context is used. I'm not sure why you didn't need it, but that might have been the exception and not the rule.

Also, I'm not sure specifically with the react-native-navigation, but the correct approach should be to wrap your app with ActionSheetProvider and then use connectActionSheet on each component. It might be as simple as changing the order of when you call the connectActionSheet among other HOC methods...

// instead of 
const ConnectedComponent = anotherHoc(connectActionSheet(MyComponent));

// try
const ConnectedComponent = connectActionSheet(anotherHoc(MyComponent));
danieltorrecillas commented 3 years ago

As another data point, I also found that I didn't have to

import { connectActionSheet } from '@expo/react-native-action-sheet'

class App extends React.Component {
  /* ... */
}

const ConnectedApp = connectActionSheet(App)

export default ConnectedApp

I just had to
1. Wrap your top-level component with <ActionSheetProvider />

and then

import { useActionSheet } from '@expo/react-native-action-sheet'

export default function App () {
  const { showActionSheetWithOptions } = useActionSheet();
  /* ... */
}

and I'm seeing the behavior I expect to see.

react v16.13.1 react-native v0.63.4 @expo/react-native-action-sheet v3.8.0

bradbyte commented 3 years ago

That's correct, the hook is an alternative to using the HOC. But in both cases you need the provider to wrap your app.