SteffeyDev / react-native-popover-view

A well-tested, adaptable, lightweight <Popover> component for react-native
MIT License
613 stars 92 forks source link

feat(popover): expose a requestClose method to close the popover programmatically #161

Closed TPXP closed 3 months ago

TPXP commented 1 year ago

The main motivation here is shorter code and the possibility to wrap this component in a component that passes its own from value

SteffeyDev commented 1 year ago

Why do you think this should be done in the core library instead of in a wrapper in your project?

TPXP commented 1 year ago

Hmm, are you suggesting something like this instead?

import React, { useRef } from 'react';
import Popover from 'react-native-popover-view';
function App() {
  const popoverRef = useRef();
  return (
    <Popover
      ref={popoverRef}
      from={(
        <TouchableOpacity>
          <Text>Press here to open popover!</Text>
        </TouchableOpacity>
      )}>
      <TouchableOpacity onPress={() => popoverRef.current.setState({isVisible: false})}>
        <Text>Tap to close me</Text>
      </TouchableOpacity>
    </Popover>
  );
}

This would make my code less reliable because it would break if the name of the state property changes, also calling setState directly on a component feels wrong.

I could re-implement the isVisible and from logic in my wrapper but since this is already implemented in the module (I know it's here), I'd rather leave it to you 😄.

TPXP commented 3 months ago

@SteffeyDev Hello there, just wanted to give a quick heads up for this. Is there anything I can do push this forward?

SteffeyDev commented 3 months ago

@TPXP Due to how long it's been since I've worked on this, I can't even spin up my expo test project anymore, which I would need to run regression tests. I'd be fine merging this in, but I'm not sure when I will have time to get everything up to date and tested so that I can deploy a new version on NPM.

TPXP commented 3 months ago

Thanks for merging! 🙌 No worries, I can apply the patch on my side in the meantime. 👍