search

equinor / webviz-subsurface-components

Custom subsurface visualizations for use in Webviz and/or Dash.
https://github.com/orgs/equinor/projects/24
Mozilla Public License 2.0
39 stars 40 forks source link

Should interface of `ColormapLayer` extend `ExtendedLayerProps` in addition to `BitmapLayerProps`? #2310

Closed rubenthoms closed 1 month ago

rubenthoms commented 1 month ago

Should the interface of ColormapLayer extend ExtendedLayerProps in addition to BitmapLayerProps to support name? name is given in defaultProps but not a valid property according to TypeScript. Same accounts for @@type.

hkfb commented 1 month ago

I think @@type is only needed if you want to specify a layer using the JSON format, which I suppose you need if you use the Dash wrapper. In that case ColormapLayer should also extend ExtendedLayerProps

Moreover, I think the name was added in order to be able to display a name label on respective views and for the layer toggles in the toolbar, which is now obsolete. We could consider deprecating the name argument or at least making it optional.

rubenthoms commented 1 month ago

Thanks for the quick reply, @hkfb. I am currently using the name property to retrieve a layer's name when handling the picking information and creating respective readout information (readout info per layer). Of course, I could use the id and map it to the layers' names, but this comes quite handy. Or do you know of a better way to retrieve the name with the current implementation?

I could create a PR with the changes to the interface of ColormapLayer, if you agree.

hkfb commented 1 month ago

Thanks for the quick reply, @hkfb. I am currently using the name property to retrieve a layer's name when handling the picking information and creating respective readout information (readout info per layer). Of course, I could use the id and map it to the layers' names, but this comes quite handy. Or do you know of a better way to retrieve the name with the current implementation?

That makes sense, I didn't think of that use case.

I could create a PR with the changes to the interface of ColormapLayer, if you agree.

Yes, please go ahead.

hkfb commented 1 month ago

:tada: This issue has been resolved in version subsurface-viewer@1.1.1 :tada:

The release is available on GitHub release