WellLogViewer - Option to hide depth-readout from viewer overlay
The Well-log viewer renders a small black box with the currently hovered or pinned log depth. This element is currently not needed for our use webviz, so it would be nice if there was an option that allowed us to disable it (for now, we're applying CSS to remove it).
One possibility is to have a callback props. IMO; there are 2 possible strategies:
-
Props is callback only If callback is provided, use it. If none, use no readout. The API should provided a default callback implementation.
-
Props is bool | callback If callback is provided, use it. If boolean, display or hide default implementation.
This prop, were you thinking it'd be checked in WellLogView.tsx, specifically in this method then?
function initOverlays(instance: LogViewer, parent: WellLogView) {
// ...
addReadoutOverlay(instance, parent);
// ...
}
And then update it to something like this?
function initOverlays(instance: LogViewer, parent: WellLogView, readoutOverlay?: Callback | boolean ) {
// ...
if (/* readoutOverlay is callback /*) {
readoutOverlay(instance, parent)
} else if (readoutOverlay !== false) {
addDefaultReadoutOverlay(instance, parent);
}
// ...
}
But looking into the code in WellLogView, isn't the way we're building these overlay elements very non-react like? We have a series of addOverlay functions, that uses a ref to the container div, and manually injects the elements and styles with native html logic... wouldnt it be more correct to change the overlay elements to be made with proper React components, as opposed to the callbacks, as you propose?
In regards to your proposed strategies, I'd say that option 1 is the cleanest (but I'd wanna use React components instead of the existing callback approach). Defaults could be provided as exported "overlay widget" components, and the core WellLogViewer component could have all these widgets added by default (the same way it default's to the right-sided layout unless)
So on the top level, you'd configure it like this, I guess?
<WellLogViewer
overlayWidgets={(logController) =>
(<>
<defaultWellPickWidget controller={logController} />
<myCustomReadoutWidget controller={logController} />
</>)}
{...otherOptionsAndListeners}
/>
Hey, I made an attempt to fix the issue. I added a new boolean prop to prevent the injection of the redout overlay (I didn't implement the callback, though).
I'll wait for your feedback to see if this is sufficient, or if the solution needs to be rethought.
Thanks!
https://github.com/equinor/webviz-subsurface-components/pull/2663
In my opinion, we should differentiate the API (ie. props) and the implementation.
The API is quite important, as we need to be consistent and ensure backward compatibility ! On the other hand, implementation can be changed without impact for the client code.
On the API, I don't like to have multiple props interfering and controlling the same topics.
Currently, the onContentSelection is a callback which gets called whenever the selection changes (a really poor name, which means whatever is under the mouse - moving the mouse changes the depth and possibly the track...).
If no onContentSelection, the default readout (red box) is displayed:
If onContentSelection is specified, the default readout is NOT displayed, and the callback is expected to handle the display itself (but the depth readout is still displayed):
We could enlarge and apply this for the depth readout for consistency, instead of adding a new property. What do you think ?
@w1nklr Thanks for taking the time to show the big picture about this issue. I think I should study the component further based on what you mentioned and implement other solution; I agree with the points you raised.