Skip to content

Fill vertical space in linked output & sidecar views #643

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

SylvainCorlay
Copy link
Contributor

@SylvainCorlay SylvainCorlay commented Apr 18, 2025

Currently, when displaying a GISDocument widget, it will always have a fixed height of 600px. This is problematic when opening the widget in a sidecar or a "linked output" view, as the widget will either take up a small portion of the height of the container or it will extend beyond the bottom of the container and require the user to scroll.

image
image

This PR sets the height to 100% in both of those contexts.

"Linked output" view can be opened by right-clicking the gutter space to the left of a widget and selecting "Create new view for cell output".

Sidecar view can be opened with jupyterlab-sidecar.

Extracted from PR #340.


📚 Documentation preview: https://jupytergis--643.org.readthedocs.build/en/643/
💡 JupyterLite preview: https://jupytergis--643.org.readthedocs.build/en/643/lite

Copy link
Contributor

Binder 👈 Launch a Binder on branch SylvainCorlay/jupytergis/mirrored-view

@arjxn-py arjxn-py added the enhancement New feature or request label Apr 18, 2025
Copy link
Contributor

Integration tests report: appsharing.space

Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mfisher87 mfisher87 changed the title Update CSS to handle filling space in linked output Update CSS to handle filling space in linked output & sidecar views Apr 18, 2025
@mfisher87 mfisher87 changed the title Update CSS to handle filling space in linked output & sidecar views Update CSS to fill vertical space in linked output & sidecar views Apr 18, 2025
@mfisher87 mfisher87 changed the title Update CSS to fill vertical space in linked output & sidecar views Fill vertical space in linked output & sidecar views Apr 18, 2025
Copy link
Member

@mfisher87 mfisher87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but I'm getting an empty map view with this change. The toolbar is visible (sometimes), but the map is not. Adding new layers through the toolbar updates the ydoc, but the map remains blank. Tested in firefox and chromium, same result.

Back on the sidecar-experiment branch, I can still call explore("some_geo.json") and get a working sidecar panel. However, now opening cell output in new view is no longer working. Sometimes the toolbar shows up, sometimes it doesn't. And the map is never showing up (except when using explore(...)). Is anyone else able to reproduce?

This was absolutely not happening when I demoed yesterday, and nothing has changed... I'm extremely confused.

In the panes without toolbars, the expected divs aren't even present. In the panes with toolbars, manually changing the height from 100% to any pixel value in the browser debug tools causes the map to display correctly.

image

I used examples/Notebook.ipynb and examples/jgis.ipynb to reproduce this behavior. I tried both opening a new view with right click on the gutter to the left, and manually creating sidecars 🤯

These behaviors seem contradictory. If it's the CSS, how come only opening a sidecar from the Python API is working and manually opening a sidecar to display a doc is not? If it's not the CSS, how come changing height from 100% to 600px fixes the map?

@SylvainCorlay
Copy link
Contributor Author

I opened jupyterlab/jupyterlab#17487 upstream in core lab so that we get the correct behavior. It also fixes the behavior in ipyleaflet.

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🚀

Should we merge this since the other mentioned issue will be fixed upstream?

Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unblocking it :)

@martinRenou martinRenou merged commit 7c36ce3 into geojupyter:main Apr 22, 2025
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants