Added clickFeatureAction config to the Layer Level #2488
Conversation
This reverts commit 702e1a9.
There was a problem hiding this comment.
Great work on the click feature implementation! I see you've effectively used optional chaining (.?) and handled different scenarios well. I tested setting the clickFeatureAction to "showDetails" on the Geohash layer in the config for the map in the arctic theme locally. I just temporarily added a line around here with clickFeatureAction: "showDetails", but it didn't seem to work. When I clicked a geohash tile, the map did not zoom, but it did not show the details panel either.
I believe it's becuase we are also checking for the value of the Map's clickFeatureAction in the MapView here. We only render the feature details panel if the clickFeatureAction is set to "showDetails" at the map level! The details panel is hidden until a feature is clicked on the map, then the contents of the panel is updated and the panel is shown. Before, we were expecting that this would never happen if the map is set to "zoom", but now we can have the details panel show up if any layer in the map is set to "showDetails".
With these changes, we can render the panel either way and not check anymore what the map-level clickFeatureAction is set to. Could you please remove the check for the map-level clickFeatureAction in the MapView and test if the details panel shows up when the Geohash layer is set to "showDetails"? I'd be happy to walk through this idea and discuss it further if you'd like!
src/js/models/maps/MapInteraction.js
Outdated
| @@ -28,7 +28,7 @@ define([ | |||
| * @since 2.27.0 | |||
| * @extends Backbone.Model | |||
| */ | |||
| var MapInteraction = Backbone.Model.extend( | |||
| let MapInteraction = Backbone.Model.extend( | |||
There was a problem hiding this comment.
Thanks for changing from var! One day we'll get them all. In this case, since the value of MapInteraction never changes, we should use const rather than let.
| let MapInteraction = Backbone.Model.extend( | |
| const MapInteraction = Backbone.Model.extend( |
| @@ -108,6 +108,9 @@ define([ | |||
| * from the layer list. | |||
| * @property {boolean} [showOpacitySlider = true] Set to true to show opacity slider | |||
| * for the layer. | |||
| * @property {boolean} [clickFeatureAction = null] The action to take when a user clicks on a feature on the layer. The | |||
There was a problem hiding this comment.
| * @property {boolean} [clickFeatureAction = null] The action to take when a user clicks on a feature on the layer. The | |
| * @property {"showDetails"|"zoom"} [clickFeatureAction = null] The action to take when a user clicks on a feature on the layer. The |
src/js/views/maps/LayerDetailView.js
Outdated
| * @property {string} [clickFeatureAction] - The default | ||
| * action to take when a user clicks on a feature on the layer. The | ||
| * available options are "showDetails" (show the feature details in the | ||
| * sidebar) or "zoom" (zoom to the feature's location). |
There was a problem hiding this comment.
I think this was committed by accident?
- unit tests for zoom and showDetails Issue NCEAS#2187 - resolved unit test issues
No description provided.