search

VEuPathDB / web-monorepo

A monorepo that contains all frontend code for VEuPathDB websites
Apache License 2.0
2 stars 0 forks source link

hide left panel and map viz at SAM when map events occur #743

Closed moontrip closed 5 months ago

moontrip commented 6 months ago

This addresses https://github.com/VEuPathDB/web-monorepo/issues/734. I think that in conjunction with left panel, it is better to close opened map Viz as well, thus this PR included that part as well.

Here is a video shot for a demo:

https://github.com/VEuPathDB/web-monorepo/assets/12802305/c9ce92c7-e119-4318-abfc-1a2c6d51bc60

moontrip commented 5 months ago

Hi @bobular your feedbacks at Slack are addressed so it would be great if you can check out.

update: I noticed a bug concerning popup flash you mentioned before. It is now fixed so it would be great if you can check this out as well.

moontrip commented 5 months ago

@moontrip I made some comments recommending a different approach.

The upshot is that MapVEuMap should take callback props for each event, and MapAnalysis should pass a callback that updates the appState so that isSidePanelExpanded = false.

Hi @dmfalke 👋 Thank you for your suggestions. I will test them next week 👍 Have a great weekend!

dmfalke commented 5 months ago

Hi @dmfalke 👋 Thank you for your suggestions. I will test them next week 👍 Have a great weekend!

🤘

moontrip commented 5 months ago

Hi @dmfalke and @bobular

I have addressed your feedbacks with some changes. One question I have is that actually we do not necessarily pass three props such as onDrag, onZoom, and onClick because a map event (drag, zoom, or click) is detected at corresponding MapVEuMapEvents in the MapVEuMap component. So, perhaps we can simply pass single prop such as setIsSidePanelExpanded(), to MapVEuMapEvents /MapVEuMap?

dmfalke commented 5 months ago

Hi @dmfalke and @bobular

I have addressed your feedbacks with some changes. One question I have is that actually we do not necessarily pass three props such as onDrag, onZoom, and onClick because a map event (drag, zoom, or click) is detected at corresponding MapVEuMapEvents in the MapVEuMap component. So, perhaps we can simply pass single prop such as setIsSidePanelExpanded(), to MapVEuMapEvents /MapVEuMap?

I think we should keep the three props. That gives us flexibility in the future, in case we want to change which events cause the side panel to be collapsed.

moontrip commented 5 months ago

Hi @dmfalke and @bobular I have addressed your feedbacks with some changes. One question I have is that actually we do not necessarily pass three props such as onDrag, onZoom, and onClick because a map event (drag, zoom, or click) is detected at corresponding MapVEuMapEvents in the MapVEuMap component. So, perhaps we can simply pass single prop such as setIsSidePanelExpanded(), to MapVEuMapEvents /MapVEuMap?

I think we should keep the three props. That gives us flexibility in the future, in case we want to change which events cause the side panel to be collapsed.

Thank @dmfalke for your comment and suggestions. I applied them to the new commit and checked it worked out 👍

moontrip commented 5 months ago

Thanks @bobular and @dmfalke for your reviews and feedbacks 👍