natysoz/expo-images-picker

Use with header from StackNavigator

shanedaugherty opened this issue · 9 comments

We would like to use this with the default header from @react-navigation/native, is there a way to do that already that maybe we are missing?

From the looks of things to make that possible we would either need to:

  • add an onChange prop that can be used to hoist the state which contains the selected items outside of the component
  • perform a refactoring that moves all of the settings into a hook, for example:
import { AssetsSelector } from 'expo-images-picker'
import { useNavigation } from '@react-navigation/native'
import { Button } from 'react-native'

const MyView = () => {
  const t = useTranslation()
  const navigator = useNavigator()

  const assetsSelectorContext = useAssetsSelector({
    Settings={widgetSettings}
    Errors={widgetErrors}
    Styles={widgetStyles}
    Resize={widgetResize}  
    Navigator={widgetNavigator}
    CustomNavigator={customNavigator}
  })

  const onSuccess = (data: Asset[]) => {
    //... here we now have access to the `assetSelectorContext` which can contain the selected item state
  }

  // making this all up for the sake of time, but you get the idea ;) 
  // when the selected items state changes, update the built in header
  useEffect(() => {
    navigator.setOptions({
      title: `selected: ${assetSelectorContext?.selected?.length ?? 0}`,
      headerRight: () => (
          <Button 
             disabled={assetSelectorContext?.selected?.length < 1} 
              onPress={onSuccess}>
                  Continue
         </Button>
       )
    })
  }, 
[assetSelectorContext?.selected])
 
  return (<AssetsSelector {...assetsSelectorContext} />)
}

this useEffect might kill performances , did you try fork and do those changes ?

Hey @natysoz!

I did not try the example posted above because it requires a bit more work. It would require us to refactor the codebase to expose a hook that can be used to access all of the state / functions.

If we do go ahead with that approach, I would not worry about the performance of the useEffect in the example. It is a pretty lightweight effect that will only run when the selected items change. Keep in mind, this useEffect is in the application which is using the library -- not in the library itself.

I did go ahead and open a PR that adds an onChange prop. This was an easy change that allows a consumer of this library to access the state of which items are selected outside of the component. I have forked it and am using it in my project!

we can create a custom hook , like useSelectedImages() where this hook return the selected items,but yhea it require to expose this function and bind it inside and along the code

Yep yep. I personally will not have the time to add this hook for a couple of months.

This pull request adds an onChange prop which also solves the same problem but in a different way: #37

I suggest we do a minor release with the onChange prop and save the hook refactor for the next major version. What are your thoughts?

i read the pr i dont understand what ur trying to return there on the onChange function ?

@natysoz the onChange function allows the component state to be hoisted outside of the component. For example

const MyView = ()=> {
  const [selectedImages, setSelectedImages] = useState<Asset>([])

  // I have access to the `selectedImages` state here now, outside of the component 

  return (
    <>
      {selectedImages?.length} selcted
      <ImagePicker onChange={setSelectedImages} />
    </>
  )
}

Seem like a good enhancement and provide much more flexibility. Looking forward to use it after PR merged

ill work on such hook soon as im busy with new kid :)
if someone wanna contribute in the meanwhile its not so hard to make

just need to test the performances

added to the feature list