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

Switch to OpenLayers for the main view #112

Merged
merged 47 commits into from
Sep 4, 2024

Conversation

gjmooney
Copy link
Collaborator

@gjmooney gjmooney commented Aug 27, 2024

Super rough draft of adding an OpenLayers viewer and fancy color things.

geotiff_color_bands

Some notes, and things for future PRs:

  • OpenLayers doesn't have a video layer like MapLibre, so we lose those for now
  • OL has a separate source for TileJSON resources, so that will need to be implemented (With MapLibre we could enter them the same as z/x/y URLs)
  • This only supports filtering for GeoJSON sources right now
  • GeoTiff sources can take multiple URLs but I don't know if RJSF forms are capable of adding fields dynamically
  • Right now the fancy color stuff is only possible for GeoTiff sources

Issues:

  • Creating a new JGIS file and adding more than one layer will cause the map to go black, reordering the layers or reloading the page fixes it, and it doesn't happen when adding layers to existing JGIS files.

@gjmooney gjmooney added the enhancement New feature or request label Aug 27, 2024
Copy link
Contributor

Binder 👈 Launch a Binder on branch gjmooney/jupytergis/open_layers_support

@gjmooney gjmooney marked this pull request as ready for review September 3, 2024 11:39
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.

Thank you for this! It's quite a huge PR 🚀

  • I think we're missing an @import url('ol/ol.css'); in our CSS, maybe in packages/base/style/base.css. OpenLayers is not styled properly.
  • The brush icon in the toolbar is mal-positioned I believe. Because for now this button only makes sense for Geotiff layers (or does it not?). I would put it in the properties panel for the geotiff layer when clicking on the layer.
  • When selecting the geotiff layer in geotiff.jGIS file, the properties panel seems confused about the form being generated and show lots of errors
  • the pmtiles-vector.jGIS example does not show the vector pmtiles layer anymore
  • I would remove the console.log calls here and there (mostly in packages/base/src/dialogs/colorExpressionDialog.tsx)

python/jupytergis_lab/src/index.ts Outdated Show resolved Hide resolved
python/jupytergis_lab/src/index.ts Outdated Show resolved Hide resolved
packages/base/src/mainview/mainViewOl.tsx Outdated Show resolved Hide resolved
packages/base/src/toolbar/widget.tsx Outdated Show resolved Hide resolved
packages/base/src/formbuilder/objectform/baseform.tsx Outdated Show resolved Hide resolved
packages/base/src/toolbar/widget.tsx Outdated Show resolved Hide resolved
packages/base/package.json Outdated Show resolved Hide resolved
@gjmooney
Copy link
Collaborator Author

gjmooney commented Sep 3, 2024

* When selecting the geotiff layer in `geotiff.jGIS` file, the properties panel seems confused about the form being generated and show lots of errors

I'm not sure how to handle this, I tried hiding the color field, but the errors still show up.

@martinRenou
Copy link
Member

I'm not sure how to handle this, I tried hiding the color field, but the errors still show up.

Yes, let's handle this in a follow-up PR with a new fancy color widget

@martinRenou
Copy link
Member

martinRenou commented Sep 4, 2024

bot please update snapshots!! pretty please

@martinRenou martinRenou changed the title Add OpenLayers support Switch to OpenLayers for the main view Sep 4, 2024
@martinRenou martinRenou closed this Sep 4, 2024
@martinRenou martinRenou reopened this Sep 4, 2024
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 Greg! Let's go with this and iterate.

We'll need to rework the color selection widgets in the properties panel

@martinRenou martinRenou merged commit 64b0b7f into geojupyter:main Sep 4, 2024
10 checks passed
brichet added a commit to brichet/jupytergis that referenced this pull request Sep 6, 2024
martinRenou pushed a commit that referenced this pull request Sep 6, 2024
* Remove the lat/long from options and use the extent instead

* update python test

* Use extent in open layer (after merging #112)

* Update examples with extent option

* Fix python test

* Set the extent as optionnal, use the lat/long by default

* lint
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.

2 participants