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

Display current visible layers status #159

Merged

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Jan 4, 2023

Signed-off-by: Junqiu Lei [email protected]

Description

Display current visible layer information on the layer control panel.

  • When the layer zoom range in current zoom level, its layer icon will be colorful, else it will be text color.
  • When the layer zoom range is out of current zoom level, move mouse to layer name, the tooltip will show it's out of zoom range.

Demo

Screen.Recording.2023-01-06.at.3.53.36.PM.mov

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@junqiu-lei junqiu-lei requested a review from a team January 4, 2023 08:13
@junqiu-lei junqiu-lei self-assigned this Jan 4, 2023
@vamshin
Copy link
Member

vamshin commented Jan 4, 2023

Should we make the Layer text name also Bold for active layers?

@junqiu-lei
Copy link
Member Author

Should we make the Layer text name also Bold for active layers?

I tried to make active layers name bold, but looks not much consistent in the list. In order to be easier to distinguish by the icon. I changed the inactive layer icon color to be totally greyed.

Only grey out inactive layer icon:
image

Grey out inactive layer icon and bold active layer name:
image

Comment on lines 337 to 338
`, Index pattern: ${layer.source.indexPatternRefName}` +
`, Geo field: ${layer.source.geoFieldName}`
Copy link
Member

@VijayanB VijayanB Jan 6, 2023

Choose a reason for hiding this comment

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

why do we need these additional details? IMO, lets say, layer is invisible outside zoom levels x - y as content and keep layer name as title.

Copy link
Member Author

@junqiu-lei junqiu-lei Jan 6, 2023

Choose a reason for hiding this comment

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

Updated

Signed-off-by: Junqiu Lei <[email protected]>
Signed-off-by: Junqiu Lei <[email protected]>
if (zoom < layer.zoomRange[0] || zoom > layer.zoomRange[1]) {
return `Layer is not visible outside of zoom range ${layer.zoomRange[0]} - ${layer.zoomRange[1]}`;
} else {
return `Layer is visible within zoom range ${layer.zoomRange[0]} - ${layer.zoomRange[1]}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This single message can cover both cases?
This layer is visible only with zoom range ${layer.zoomRange[0]} - ${layer.zoomRange[1]};

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, currently we will only need tooltip for invisible layers, will add more content for visible layers.

@junqiu-lei junqiu-lei merged commit b46e7f4 into opensearch-project:feature/new-maps Jan 7, 2023
@junqiu-lei junqiu-lei deleted the current_layers branch January 7, 2023 00:09
@kgcreative
Copy link
Member

Can we also update the show/hide "eye" icon to properly display visible/invisible current status?

@junqiu-lei
Copy link
Member Author

Can we also update the show/hide "eye" icon to properly display visible/invisible current status?

Hi @kgcreative , the "eye" icon button was designed to control layer visible status to maplibre for this layer. If we also update the eye icon status based on whether layer range is in current zoom level or not, it might make the eye icon status changed wired.

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.

5 participants