Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: isOpen for Popup #317

Closed
stereobooster opened this issue Apr 23, 2017 · 12 comments
Closed

Feature request: isOpen for Popup #317

stereobooster opened this issue Apr 23, 2017 · 12 comments

Comments

@stereobooster
Copy link

I have list of places. For each place I want to have marker. If I click on a place in a list I want to show popup for correspondent marker for correspondent place, to achieve this I will set state of parent component to selected place and then rerender marker with one of the Popups opened. Is this the way it should be done? If yes I can do simple PR with isVisible property Popup

@PaulLeCam
Copy link
Owner

Please refer to Leaflet's documentation, there is the isOpen() method that seem to provide what you need.

@stereobooster
Copy link
Author

it is not. I do not want to check visibility - I want to control visibility in declarative manner, like this:

<Popup isOpen={true}>
<Popup isOpen={false}>

I know that this lib integrates with third-party, so not all React paradigms are feasible, that's why I'm asking if this is good idea or not

@stereobooster
Copy link
Author

As of now Marker-Popup pair works as uncontrolled component e.g. it maintain it's own internal state and this state updated directly with user interaction, not through react setState.

What I'm suggesting is to implement controlled component e.g. Popup will open if you set state or property externally.

@PaulLeCam
Copy link
Owner

As far as I know It's not something supported by Leaflet itself so it won't be implemented in this library either, but you can always create a custom component to implement the logic you need.

@stereobooster
Copy link
Author

This is not concern of Leaflet, this is concern of React implementation. Html input does not support this behaviour out of the box too, instead React component use event handler and e.preventDefault to achieve Reactive way to do it.

@PaulLeCam
Copy link
Owner

I understand what you're asking but this is out of the scope of this library.
One important thing to consider is that Leaflet maintains its own state and handles the interactions with the DOM, React only provides the component abstraction, see the "how it works" section of the documentation for more information.

@stereobooster
Copy link
Author

I get your point, way to do this is to implement "plugin". Just one note

One important thing to consider is that Leaflet maintains its own state and handles the interactions with the DOM

html <Input> maintains its own state too, but React folks take step further and wrote wrapper to adapt it to React workflow. This is not a general stopper, this is just a choice.

@stereobooster stereobooster changed the title Feature request: isVisible for Popup Feature request: isOpen for Popup Apr 28, 2017
@ghost
Copy link

ghost commented May 16, 2017

@stereobooster I've managed to open the popup with the following code (just the prototype):

const targets = targetList.map(target => (
      <Marker position={[target.lat, target.lng]} key={target.id}>
        <Popup ref={ref => !target.valid && setTimeout(() => x(ref, [target.lat, target.lng]))}>
          <Target target={target} />
        </Popup>
      </Marker>));
function x(ref, pos) {
  ref.context.map.openPopup(ref.leafletElement, pos);
}

I know ... setTimeout in reder... Didn't find a better implementation yet.

@pvhieuit
Copy link

I found this in jsfiddle

// Create your own class, extending from the Marker class.
class ExtendedMarker extends Marker {
  // "Hijack" the component lifecycle.
  componentDidMount() {
    // Call the Marker class componentDidMount (to make sure everything behaves as normal)
    super.componentDidMount();
    
    // Access the marker element and open the popup.
    this.leafletElement.openPopup();
  }
}
  render() {
    const position = [this.state.lat, this.state.lng];
    return (
      <Map center={position} zoom={this.state.zoom}>
        <TileLayer
          attribution='&copy; <a href="http://osm.org/copyright">OpenStreetMap</a> contributors'
          url='http://{s}.tile.osm.org/{z}/{x}/{y}.png'
        />
        <ExtendedMarker position={position}>
          <Popup>
            <span>A pretty CSS3 popup. <br/> Easily customizable.</span>
          </Popup>
        </ExtendedMarker>
      </Map>
    );
  }

@mikedeng
Copy link

mikedeng commented May 24, 2019

I agree with @pvhieuit, but it will report an error in react-leaflet v2, this error will appear:

TypeError: Super expression must either be null or a function, not object

and the solution goes here

@phanirithvij
Copy link

phanirithvij commented Dec 7, 2020

My workaround using accessing the dom (horrible but works).

Using hooks and functional components, and programmatically clicking on the marker to open/close it.

with comments
  const togglePopup = () => {
    document.querySelectorAll(".marker-x")?.[1].click();
  };

  const openPopup = () => {
    if (document.querySelector(".popupcl")) {
      // popup is already open because the popup div exists
      // that div i.e. the popup, will be removed from dom by leaflet when we close the popup
      // already open so return
      return;
    }
    // I'm using shadow (see <Marker /> below) and react-leaflet assigns the className to both the marker and the shadow
    // Must use [0] or just document.querySelector(".marker-x") if not using shadow
    document.querySelectorAll(".marker-x")?.[1].click();
  };

  const [position, setPos] = useState(null);
  useEffect(() => {
    if (position !== null) openPopup();
  }, [position]);

    <Marker
      draggable={true}
      .....
      // Need to use this icon to specify a class to act as an identifier for the marker image
      icon={
        new Icon({
          iconUrl: iconURL,
         // NOTE: If this is not needed need to use document.querySelector('.marker-x') in openPopup
          shadowUrl: shadowURL,
          // Add a custom classname so we can query it
          className: "marker-x",
        })
      }
      position={position}
    >
      {/*  Using a custom classname here to query this as well  */}
      <Popup ... className="popupcl">
         ...
       </Popup>
     </Marker>
no comments
  const togglePopup = () => {
    document.querySelectorAll(".marker-x")?.[1].click();
  };
  const openPopup = () => {
    if (document.querySelector(".popupcl")) return;
     // see above comments
    document.querySelectorAll(".marker-x")?.[1].click();
  };

  const [position, setPos] = useState(null);
  useEffect(() => {
    if (position !== null) openPopup();
  }, [position]);

  return (
    <Marker
      draggable={true}
      icon={
        new Icon({
          iconUrl: iconURL,
          shadowUrl: shadowURL,
          className: "marker-x",
        })
      }
      position={position}
    >
      <Popup className="popupcl">
       </Popup>
     </Marker>
    );

Leaflet provides these functions openpopup, closepopup, etc. so would be great if this library also provides those methods at the top level.

@danielvandermaas
Copy link

Maybe in many cases it could be better to use the Tooltip component instead of automatically opening the popup component.

https://stackoverflow.com/questions/65345016/react-leaflet-add-label-popup-next-to-marker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants