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

Improve map height #220

Merged
merged 9 commits into from
Nov 13, 2023
Merged

Improve map height #220

merged 9 commits into from
Nov 13, 2023

Conversation

vgeorge
Copy link
Member

@vgeorge vgeorge commented Nov 8, 2023

Contributes to #167, WIP.

With this change, if the user doesn't set _height to Map, the component will try to dynamically occupy the full height of the parent element.

This will make the map height when using Sidecar to occupy the full height, which is nice, but it will require the user to know that _height must be set when not using sidecar, otherwise, the widget will look like this:

Screenshot 2023-11-08 at 16 42 06

I believe we need to figure out how to set a default minHeight so the map would display well on both use cases.

cc @kylebarron

@vgeorge
Copy link
Member Author

vgeorge commented Nov 8, 2023

@kylebarron I added min-height to the widget wrapper and it seems to be working, could you please review?

@kylebarron
Copy link
Member

This doesn't seem to be working for me without sidecar in JupyterLab...
image

After I manually set the height it works again
image

src/index.tsx Outdated Show resolved Hide resolved
@kylebarron
Copy link
Member

I'm not sure if the latest change to have a single uuid div per map works either... Ideally we want to have a unique id per view not a unique id per model. For example, it still breaks if I first render a map inline and then try to move the map to a sidecar:

image

notebook code:

import geopandas as gpd
from lonboard import viz
from sidecar import Sidecar

gdf = gpd.read_file(gpd.datasets.get_path('nybb'))
map_ = viz(gdf)
map_

sidecar = Sidecar()
with sidecar:
    display(map_)

presumably it's finding the wrong div

@kylebarron
Copy link
Member

Oh smart, that looks great! Thanks!

@kylebarron kylebarron merged commit 6ced305 into main Nov 13, 2023
4 checks passed
@kylebarron kylebarron deleted the fix/map-height branch November 13, 2023 22:24
@kylebarron kylebarron mentioned this pull request Nov 26, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants