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

layerGroups popupContent popup(s) not closing on layergroup toggle-off #1771

Open
reterraform opened this issue Oct 27, 2019 · 7 comments
Open
Assignees

Comments

@reterraform
Copy link

Hello, I'm leveraging popupContent popups for some non-api-dataset externally sourced layergroups, which work fine till the layer is toggled 'off' (non-visible) from left-sidebar, leaving the popup(s) remaining open/visible w/o the layer they reference (ie orphaned). Tried closing from the LeftSidebarSectionSelector.onToggleLayerGroup property, but having no luck (can prevent popups' opening from there, but not able to close em). Also tested forcing close from client-side script (i know, hacky :/ ), but no luck there either (the emotion styling seems to make it more challenging to reference DOM objects; for me anyhoo).

Any tips re how & where to best etc greatly appreciated, as always!

@reterraform reterraform changed the title mapbox layers popupContent not closing on layer close layerGroups popupContent popup(s) not closing on layergroup toggle-off Oct 27, 2019
@reterraform
Copy link
Author

???

@goldpbear
Copy link
Contributor

goldpbear commented Nov 2, 2019

Hi-- thanks for reporting this issue. You're definitely right that a popup will stay open if its layer is switched off-- we hadn't considered that interaction but I agree it produces a weird effect.

From what I can tell there isn't a ready way to automatically close popups in the current implementation if their parent layer is switched off. I believe what we'd need to do is track which layer the current popup belongs to, then have a check in the componentDidUpdate lifecycle method of the MainMap component to determine if that layer has been switched off and, if so, hide the popup.

You can see how the popup's content and position get set here:

if (
feature &&
this.props.mapLayerPopupSelector(feature.layer.id) &&
// When `center.x` matches `mouseX` and `center.y` matches `mouseY`, the
// user has clicked in place (as opposed to clicking, dragging, then
// releasing). We only want to render popups on in-place clicks.
//
// We wouldn't need to worry about tracking this information
// if we used the `onClick` listener, but as explained above we avoid
// `onClick` for performance reasons.
evt.center.x === this.mouseX &&
evt.center.y === this.mouseY
) {
const popupContent = this.parsePopupContent(
this.props.mapLayerPopupSelector(feature.layer.id),
feature.properties,
);
// Display popup.
this.setState({
popupContent,
popupLatitude: evt.lngLat[1],
popupLongitude: evt.lngLat[0],
});
}
};

And where the popup is rendered here:

{this.state.popupContent &&
this.state.popupLatitude &&
this.state.popupLongitude && (
<Popup
latitude={this.state.popupLatitude}
longitude={this.state.popupLongitude}
closeOnClick={false}
onClose={() =>
this.setState({
popupContent: null,
})
}
anchor="bottom"
>
<div
dangerouslySetInnerHTML={{ __html: this.state.popupContent }}
/>
</Popup>

I can probably add a feature to automatically close popups in the next couple of weeks. In the meantime you can experiment with setting the closeOnClick prop of the Popup to true-- not a solution, but it gives you a larger target to click to close the popup at least.

@reterraform
Copy link
Author

Thanks for that helpful info Trevor! I'll give those ideas a look & see if I can get some traction. Meanwhile, really appreciate & look forward to that potential feature addition (especially if I continue to flail w/ my 'hack' efforts). :/

CHEERS

@goldpbear goldpbear self-assigned this Nov 5, 2019
@goldpbear
Copy link
Contributor

Sure thing-- thanks again for your interest in Mapseed! I'll try to push up a fix for the popups as soon as I can. We're wrapping up a big feature atm, but once that's done I should get some time.

@reterraform
Copy link
Author

sounds good; thanks once again!

@reterraform
Copy link
Author

Know you folks are busy with plenty of tasks, but just checking in on status of this issue. All your good works much appreciated, in any case! :)

@goldpbear
Copy link
Contributor

goldpbear commented Feb 15, 2020

Hey-- thanks for checking in! And sorry for the delay on this request.

Unfortunately at this point I don't think we can give an estimated ship date. We're working at a reduced capacity lately balancing this project with others, and it's difficult to provide an estimate. I wish it were otherwise, though.

We definitely appreciate your interest, and I'd be happy to try and recommend other tools that might be able to meet your use case more readily, if that's an option for you.

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

No branches or pull requests

2 participants