-
Notifications
You must be signed in to change notification settings - Fork 43
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
Enhance layer visibility status display #299
Conversation
Signed-off-by: Junqiu Lei <junqiu@amazon.com>
}; | ||
|
||
const layerIsVisible = (layer: MapLayerSpecification) => { | ||
const layerIsVisible = (layer: MapLayerSpecification): boolean => { | ||
if (layer.visibility === 'none') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think in other places we decides whether a layer is visible or not based on value 'visible'. Let's not add multiple definition and keep with check layer.visibility === 'visible'
as visibility check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks let me know, updated
@@ -226,14 +226,20 @@ export const LayerControlPanel = memo( | |||
}; | |||
|
|||
const getLayerTooltipContent = (layer: MapLayerSpecification) => { | |||
if (layer.visibility === 'none') { | |||
return 'Layer is hidden'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need translation here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added i18n here
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #299 +/- ##
=======================================
Coverage 91.42% 91.42%
=======================================
Files 11 11
Lines 280 280
Branches 37 37
=======================================
Hits 256 256
Misses 17 17
Partials 7 7 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Junqiu Lei <junqiu@amazon.com>
Hi @kolchfa-aws, could you help review the PR from document perspective? Thank you! |
The first tooltip is fine. Here's a suggestion for the second one: |
Thanks @kolchfa-aws suggestion. It will be: |
Yup, LGTM. |
Signed-off-by: Junqiu Lei <junqiu@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Gray out layer name and icon on invisible layer * Add i18n Signed-off-by: Junqiu Lei <junqiu@amazon.com> (cherry picked from commit 70d5f9e)
…nsearch-project#302) * Gray out layer name and icon on invisible layer * Add i18n Signed-off-by: Junqiu Lei <junqiu@amazon.com> (cherry picked from commit 70d5f9e) Co-authored-by: Junqiu Lei <junqiu@amazon.com> (cherry picked from commit 87c2422)
Description
Enhance layer visibility status display.
Priority 0: When layer visibility is none: 1) show tooltip "layer is hidden" and 2) gray out the icon and layer name
Priority 1: When layer setting zoom level range is out of current zoom level, 1) show tooltip "layer is not visible outside of zoom range a to b" and 2) gray out the icon and layer name
![image](https://user-images.githubusercontent.com/90288540/221714532-86f29dce-8230-4cab-b03b-d7a0e4230a8c.png)
Issues Resolved
Closes #298
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.