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

Add layer browser icon for geotiff #321

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gjmooney
Copy link
Collaborator

@gjmooney gjmooney commented Jan 13, 2025

Description

Adds icon for GeoTiff sources and and WebGlTile layers in the layer browser.

geotifficon

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

📚 Documentation preview: https://jupytergis--321.org.readthedocs.build/en/321/
💡 JupyterLite preview is available from the doc, by clicking on lite badge

@gjmooney gjmooney added the bug Something isn't working label Jan 13, 2025
Copy link
Contributor

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

Copy link
Contributor

Integration tests report: appsharing.space

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.

LGTM and left some thoughts

@@ -78,12 +78,14 @@ const iconObject = {
ImageSource: { iconClass: 'fa fa-image' },
VideoSource: { iconClass: 'fa fa-video' },
ShapefileSource: { iconClass: 'fa fa-file' },
GeoTiffSource: { iconClass: 'fa fa-gear' },
Copy link
Member

Choose a reason for hiding this comment

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

Gear, as in "cog"? Excellent choice ;)


RasterLayer: { icon: rasterIcon },
VectorLayer: { iconClass: 'fa fa-vector-square' },
HillshadeLayer: { icon: moundIcon },
ImageLayer: { iconClass: 'fa fa-image' },
VideoLayer: { iconClass: 'fa fa-video' },
WebGlLayer: { iconClass: 'fa fa-gear' },
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to have a dedicated icon for webGL layer? As a user, I don't think this is a meaningful classification. A webGL layer in openlayers can be a raster tile layer or a vector layer. What about showing the raster or vector icons depending on which?

Copy link
Member

Choose a reason for hiding this comment

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

+1 for a distinct icon for better identification. Since we have a dedicated handling for a webGL layer maybe using this icon could make more sense?

@martinRenou
Copy link
Member

martinRenou commented Jan 14, 2025

I have mixed feelings for the cog icon :D

  • Does QGIS use a specific icon for GeoTIFF?
  • Should we use the same icon as for classic raster layers?

EDIT: No hard feelings though, if there is a consensus on the COG icon and I'm the only one rethinking it, happy to merge and ignore my doubts 😄

@gjmooney
Copy link
Collaborator Author

I have mixed feelings for the cog icon :D

* Does QGIS use a specific icon for GeoTIFF?

* Should we use the same icon as for classic raster layers?

EDIT: No hard feelings though, if there is a consensus on the COG icon and I'm the only one rethinking it, happy to merge and ignore my doubts 😄

QGIS uses a checkerboard looking thing for tiffs, I think that's what it uses for all raster layers. There's been a lot of icon discourse this week 😂 we might want to have a discussion about it.

@arjxn-py
Copy link
Member

QGIS uses a checkerboard looking thing for tiffs, I think that's what it uses for all raster layers.

image

Yeah, neither pretty nor recognisable

@mfisher87
Copy link
Member

mfisher87 commented Jan 14, 2025

QGIS uses a checkerboard looking thing for tiffs, I think that's what it uses for all raster layers. There's been a lot of icon discourse this week 😂 we might want to have a discussion about it.

Yeah QGIS only differentiates vector and raster layers with icons, and I like the simplicity of that.

That said, I agree with @arjxn-py that the QGIS icon for raster isn't super recognizable.

For vector, fontawesome has an icon that looks pretty good for vector data: https://fontawesome.com/icons/draw-polygon?s=solid

Similar to the checkerboard icon, fontawesome results for "grid": https://fontawesome.com/search?q=grid&o=r

Or: https://fontawesome.com/icons/game-board-simple?f=sharp&s=solid ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants