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.

Sign up for GitHub

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

Jump to bottom

Add raster layer gallery #16

Merged
merged 42 commits into from
Jul 2, 2024
Merged

Add raster layer gallery #16

merged 42 commits into from
Jul 2, 2024

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Jun 24, 2024 •
edited by gjmooney
Loading

cc. @gjmooney

Add a raster layer gallery using xyzservices.

Common sense would want to create a server endpoint for accessing the gallery items, I'm not taking this approach otherwise it would not work in the case of jupyterlite.

Remaining for this PR:

  • Should we really commit the images or include them at build time?
  • Missing attribution entry and some other raster layer parameters
  • Make the layer gallery registry a plugin

@github-actions GitHub Actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch martinRenou/jupytergis/raster_layer_gallery

@davidbrochart
Copy link
Collaborator

A raster layer gallery sounds nice, but should it be implemented as a registry and populated by plugins?

@martinRenou
Copy link
Member Author

martinRenou commented Jun 25, 2024 •
edited
Loading

A raster layer gallery sounds nice, but should it be implemented as a registry and populated by plugins?

Yeah definitely 👍🏽 Added it to the TODO list of the PR

@martinRenou martinRenou added the enhancement New feature or request label Jun 25, 2024
@gjmooney gjmooney force-pushed the raster_layer_gallery branch from 51c8b8f to 4cde266 Compare June 25, 2024 12:27
@gjmooney gjmooney force-pushed the raster_layer_gallery branch 4 times, most recently from ad69338 to d13e447 Compare July 1, 2024 09:15
@gjmooney gjmooney marked this pull request as ready for review July 1, 2024 09:20
brichet
brichet reviewed Jul 1, 2024
View reviewed changes
Copy link
Collaborator

@brichet brichet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @gjmooney and @martinRenou

As a general comment, I think that we should start to add doc string when we add functions, it's always a bit easier to review with context.

Otherwise I mostly have some questions about implementation details.

packages/base/rasterlayer_gallery_generator.py Outdated Show resolved Hide resolved
packages/base/src/commands.ts Outdated Show resolved Hide resolved
packages/base/src/tools.ts Outdated Show resolved Hide resolved
python/jupytergis_lab/src/index.ts Show resolved Hide resolved
packages/base/src/layerBrowser/layerBrowserDialog.tsx Outdated Show resolved Hide resolved
packages/base/src/layerBrowser/layerBrowserDialog.tsx Show resolved Hide resolved
packages/base/src/layerBrowser/layerBrowserDialog.tsx Outdated Show resolved Hide resolved
@gjmooney gjmooney force-pushed the raster_layer_gallery branch from 173b464 to a92a688 Compare July 1, 2024 15:38
martinRenou
martinRenou commented Jul 2, 2024
View reviewed changes
packages/base/rasterlayer_gallery/raster_layer_gallery.json Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess this should be removed and not be commited?

@martinRenou
Copy link
Member Author

I just have a tiny suggestion, then I believe this is looking good and we can merge!

@martinRenou martinRenou force-pushed the raster_layer_gallery branch from a92a688 to f3dab84 Compare July 2, 2024 07:44
gjmooney added 24 commits July 2, 2024 13:35
@gjmooney
Add some attributes to source - wip
baa43c0
@gjmooney
Add new layers to layer tree
4f588d7
@gjmooney
Move layer browser css
accbb53
@gjmooney
Make layer browser a plugin
cd24969
@gjmooney
Add webpack-env types to clear ts error
1c82fc6
@gjmooney
Move layer browser css
3fb8ae3
@gjmooney
Create thumbnails at build time and add build:dev option to skip it
f579efc
@gjmooney
Fix build stuff
a505e83
@gjmooney
Add script dependecies to build action
bc428fc
@gjmooney
Remove layer tree stuff
f887c0c
@gjmooney
Add some tests
eb7e2f4
@gjmooney
Small fixes
da1137a
@gjmooney
Add remove/clear methods to registry
aaadb20
@gjmooney
Add script dependecies to requirements
65cee82
@gjmooney
Fix tests
12f8d3b
@gjmooney
Rename registry functions
4cf226c
@gjmooney
Remove raster layer gallery
b8c8f7a
@gjmooney
Make attribution optional
3253ad2
@gjmooney
Use models addLayer()
aa382e5
@gjmooney
Add some jsdoc
c9ec5eb
@gjmooney
CI test
7f681d2
@gjmooney
CI fix maybe
e6ee0f8
@gjmooney
Rework tests
21ea066
@gjmooney
Resize grid for smaller windows
4e8fb69
@gjmooney gjmooney force-pushed the raster_layer_gallery branch from 833627b to 4e8fb69 Compare July 2, 2024 11:35
martinRenou
martinRenou commented Jul 2, 2024
View reviewed changes
packages/base/src/layerBrowser/layerBrowserDialog.tsx
// The split is to get rid of the 'Layer' part of the name to match the names in the gallery
setActiveLayers(
Object.values(model.sharedModel.layers).map(
layer => layer.name.split(' ')[0]
Copy link
Member Author

Choose a reason for hiding this comment

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

I believe we should not rely on the layer names for this, we may provide the ability to rename layers then the logic here is not reliable anymore.

We may keep the "Added!" transient while the dialog is open, but as soon at it's closed it does not remember which layers were already added?

@martinRenou
Copy link
Member Author

Let's merge and iterate! Thanks Greg :)

@martinRenou martinRenou merged commit 86c9504 into geojupyter:main Jul 2, 2024
9 checks passed
@martinRenou martinRenou deleted the raster_layer_gallery branch July 2, 2024 13:05
@brichet brichet mentioned this pull request Jul 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gjmooney gjmooney gjmooney left review comments

@brichet brichet brichet left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@martinRenou @davidbrochart @brichet @gjmooney