Introduce disable tooltip on hover property #313
Conversation
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 #313 +/- ##
==========================================
+ Coverage 91.66% 91.69% +0.02%
==========================================
Files 12 12
Lines 288 289 +1
Branches 38 38
==========================================
+ Hits 264 265 +1
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. |
| export function isTooltipEnabledOnHover( | ||
| layer: MapLayerSpecification | ||
| ): layer is DocumentLayerSpecification { | ||
| return isTooltipEnabledLayer(layer) && !layer.source.disableTooltipsOnHover; |
There was a problem hiding this comment.
| return isTooltipEnabledLayer(layer) && !layer.source.disableTooltipsOnHover; | |
| if (!isTooltipEnabledLayer(layer)) { | |
| return false; | |
| } | |
| return !layer.source?.disableTooltipsOnHover; |
There was a problem hiding this comment.
I think this way the logic can be more clear, and adding ? can prevent errors if the source property is missing.
There was a problem hiding this comment.
source is not optional parameters in DocumentLayerSpecification. I assume typescript would have complained if '?' is missing. TBH, suggestion looks very verbose.
There was a problem hiding this comment.
i made this parameter is optional and added '?', but still i believe splitting in inside block is verbose. If you are strongly against it i can change it.
There was a problem hiding this comment.
Thanks on the change, I think either splitting or not is okay, IMO splitting is easier for reading the code.
Added new property that allows users to disable tooltip on hover. The default value is false to keep backward compatability. We will revisit the ideal default value based on user's feedback in later release. Signed-off-by: Vijayan Balasubramanian <[email protected]>
VijayanB
force-pushed
the
add-props-tooltip
branch
from
2078889 to
25da75c
Compare
Added new property that allows users to disable tooltip on hover. The default value is false to keep backward compatability. We will revisit the ideal default value based on user's feedback in later release. Signed-off-by: Vijayan Balasubramanian <[email protected]> (cherry picked from commit 03b0ee4)
Added new property that allows users to disable tooltip on hover. The default value is false to keep backward compatability. We will revisit the ideal default value based on user's feedback in later release. Signed-off-by: Vijayan Balasubramanian <[email protected]>
…pensearch-project#314) Added new property that allows users to disable tooltip on hover. The default value is false to keep backward compatability. We will revisit the ideal default value based on user's feedback in later release. Signed-off-by: Vijayan Balasubramanian <[email protected]> (cherry picked from commit d0ef1fd)
Description
Added new property that allows users to disable tooltip on hover. The default value is false to keep
backward compatability. We will revisit the ideal
default value based on user's feedback in later release.
Issues Resolved
#259
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.