SteffeyDev / react-native-popover-view

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

Full example not working in Expo #66

Closed jadchaar closed 4 years ago

jadchaar commented 4 years ago

I am testing out this package in Expo by running the full example, but it does not seem to be working: https://snack.expo.io/@jadchaar/popover-demo. Any ideas?

jadchaar commented 4 years ago

It is worth noting that the following debug message comes up:

Popover Warning - Can't Show - Attempted to show a Popover while another one was already showing. You can only show one Popover at a time, and must wait for one to close completely before showing a different one. You can use the onCloseComplete prop to detect when a Popover has finished closing. To show multiple Popovers simultaneously, all but one should have mode={Popover.MODE.JS_MODAL}. Once you change the mode, you can show as many Popovers as you want, but you are responsible for keeping them above other views.

But it does not make much sense to me since I only have a single popover trying to show and it is using the mode "tooltip".

jadchaar commented 4 years ago

Update: when I opened up Expo in a new tab it seems to now be working... That said, the full example still has a typo since the class ends with }); when it should just be } 😄 .

jadchaar commented 4 years ago

Sorry for the constant barrage, but I tried changing this to a functional component using hooks and it seems to break since it does not think that touchableRef is a React component.

import React, { Component, useState, useRef, useEffect } from 'react';
import Popover from 'react-native-popover-view';
import {
  AppRegistry,
  StyleSheet,
  Text,
  TouchableHighlight,
  View,
} from 'react-native';

export default function App() {
  const [isVisible, setIsVisible] = useState(false);

  let touchableRef = useRef(null);

  return (
    <View style={styles.container}>
      <TouchableHighlight ref={touchableRef} style={styles.button} onPress={() => setIsVisible(true)}>
        <Text>Press me</Text>
      </TouchableHighlight>

      <Popover
        isVisible={isVisible}
        fromView={touchableRef}
        mode="tooltip"
        debug={true}
        popoverStyle={styles.customPopoverStyles}
        verticalOffset={-3}
        placement="top"
        onRequestClose={() => setIsVisible(false)}>
        <Text>I'm the content of this popover!</Text>
      </Popover>
    </View>
  );
}

var styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: 'rgb(43, 186, 180)',
  },
  button: {
    borderRadius: 4,
    padding: 10,
    marginLeft: 10,
    marginRight: 10,
    backgroundColor: '#ccc',
    borderColor: '#333',
    borderWidth: 1,
  },
  customPopoverStyles: {
    padding: 20,
  },
});

Check out the Expo here: https://snack.expo.io/@jadchaar/popover-demo---hooks

SteffeyDev commented 4 years ago

For the last issue, try passing in touchableRef.current and see if that works

SteffeyDev commented 4 years ago

Thanks for pointing out the issue in the docs, I’ll get that fixed ASAP!

SteffeyDev commented 4 years ago

You can also check out my test project react-native-popover-view-test-app exp://exp.host/@steffeydev/react-native-popover-view-test-app for a working example, and also see the underlying code on github as well

SteffeyDev commented 4 years ago

Obviously that’s not written for hooks yet though

jadchaar commented 4 years ago

For the last issue, try passing in touchableRef.current and see if that works

Sadly that did not work :(. According to the useRef docs this is an appropriate usage pattern: https://reactjs.org/docs/hooks-reference.html#useref.

Thanks for fixing the docs!

SteffeyDev commented 4 years ago

Ok, for now just switch back to using a class wrapper around that component then and I will investigate this week to see if I can get it working with hooks

SteffeyDev commented 4 years ago

This might explain the issue:

Keep in mind that useRef doesn’t notify you when its content changes. Mutating the .current property doesn’t cause a re-render. If you want to run some code when React attaches or detaches a ref to a DOM node, you may want to use a callback ref instead.

Initially the ref is null, and the view is not re-rendered when it is set, so the Popover is never given a non-null ref unless it is re-rendered for a different reason.

Consider something like this (not tested but hopeful):

export default function App() {
  const [isVisible, setIsVisible] = useState(false);
  const [touchable, setTouchable] = useState(null);

  const touchableRef = useCallback(node => {
    if (node !== null) {
      setTouchable(node);
    }
  }, []);

  return (
    <View style={styles.container}>
      <TouchableHighlight ref={touchableRef} style={styles.button} onPress={() => setIsVisible(true)}>
        <Text>Press me</Text>
      </TouchableHighlight>

      <Popover
        isVisible={isVisible}
        fromView={touchable}
        mode="tooltip"
        debug={true}
        popoverStyle={styles.customPopoverStyles}
        verticalOffset={-3}
        placement="top"
        onRequestClose={() => setIsVisible(false)}>
        <Text>I'm the content of this popover!</Text>
      </Popover>
    </View>
  );
}

See https://reactjs.org/docs/hooks-faq.html#how-can-i-measure-a-dom-node for more background on this example.

jadchaar commented 4 years ago
export default function App() {
  const [isVisible, setIsVisible] = useState(false);
  const [touchable, setTouchable] = useState(null);

  const touchableRef = useCallback(node => {
    if (node !== null) {
      setTouchable(node);
    }
  }, []);

  return (
    <View style={styles.container}>
      <TouchableHighlight ref={touchableRef} style={styles.button} onPress={() => setIsVisible(true)}>
        <Text>Press me</Text>
      </TouchableHighlight>

      <Popover
        isVisible={isVisible}
        fromView={touchable}
        mode="tooltip"
        debug={true}
        popoverStyle={styles.customPopoverStyles}
        verticalOffset={-3}
        placement="top"
        onRequestClose={() => setIsVisible(false)}>
        <Text>I'm the content of this popover!</Text>
      </Popover>
    </View>
  );
}

That does in fact work :).

SteffeyDev commented 4 years ago

Great! I’m working on refactoring and will bring better support for hooks soon.