plotly / dash-cytoscape

Interactive network visualization in Python and Dash, powered by Cytoscape.js
https://dash.plot.ly/cytoscape
MIT License
587 stars 120 forks source link

Bugfix/update maxzoom cyleaflet #216

Closed Farkites closed 2 months ago

Farkites commented 2 months ago

About

This PR adds two bug fixes for the issues #208, #207, and #205 regarding the CyLeaflet AIO component. ## Description of changes #205: - A new `dcc.Store` has been added to the AIO component containing the data of the last leaflet `extent` state so that when the leaflet tiles contain a new `maxZoom` the `extent` isn't lost. - I have substituted the dictionary to map leaflet to cytoscape max zooms with a function that approximates the values in that dictionary and extends them to any maxZoom. - A callback that updates cytoscape's maxZoom when the children of leaflet change has been added. - The cytoscape maxZoom has been added as an input to the `updateLeafBounds` callback. - The demo `usage_cy_leaflet_aio.py` has been updated to include a leaflet maxZoom input. #208: - The callback `TransformElements` allows duplicate outputs for the cytoscape elements. - A callback that updates AIO `dcc.Store` `elements` whenever cytoscape elements change has been added. - The demo `usage_cy_leaflet_aio.py` has been updated to include a number of elements for cytoscape input. #207: - Recalculate latitude and longitude on the callback that updates `elements` `dcc.Store`. ## Pre-Merge checklist - [x] The project was correctly built with `npm run build:all`. - [x] If there was any conflict, it was solved correctly. - [x] All changes were documented in CHANGELOG.md. - [x] All tests on CircleCI have passed. - [x] All Percy visual changes have been approved. - [x] At least one person has :dancer:'d the pull request. ## Reference Issues Closes #208 Closes #205 Closes #207
emilykl commented 2 months ago

@Farkites I've noticed something that looks like a bug -- when the maxZoom is decreased, the map doesn't zoom out, which results in the map disappearing because now we're zoomed in greater than the max zoom.

https://github.com/plotly/dash-cytoscape/assets/4672118/a1bb9b08-3e2c-40c4-8d30-a40a41f5e917

I know Leaflet is finicky and maybe there's no easy fix, but IMO the ideal would be that the map automatically zooms out to match the new max zoom.

Farkites commented 2 months ago

@Farkites I've noticed something that looks like a bug -- when the maxZoom is decreased, the map doesn't zoom out, which results in the map disappearing because now we're zoomed in greater than the max zoom.

Screen.Recording.2024-05-13.at.6.07.25.PM.mov I know Leaflet is finicky and maybe there's no easy fix, but IMO the ideal would be that the map automatically zooms out to match the new max zoom.

I found a way to fix this. On the event layoutstop I'm checking if the zoom is either over the max zoom or below the min zoom and calling cy.fit() if it's the case.

I haven't found a way to do this from the clientside callbacks since zoom is not a valid input/output.

Farkites commented 2 months ago

One more thing to check before merging -- can you confirm that the scenario described in #205 works properly now? I.e., switching out the leaflet children for a tile layer with a different maxZoom updates the behavior of the CyLeaflet as expected?

Yes, it works as expected!