-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement Well-log viewer module MVP #794
Implement Well-log viewer module MVP #794
Conversation
* Supports continuous curves from SSDL endpoint * Settings are persisted across reloads
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.
Nice work! 👍
Admittedly, I have quite a few comments and suggestions but this is mostly due to the PR having gotten quite large.
By the way, it seems you added the preview image to the old branch.
frontend/src/modules/WellLogViewer/view/SubsurfaceLogViewerWrapper.tsx
Outdated
Show resolved
Hide resolved
frontend/src/modules/WellLogViewer/view/SubsurfaceLogViewerWrapper.tsx
Outdated
Show resolved
Hide resolved
frontend/src/modules/WellLogViewer/view/SubsurfaceLogViewerWrapper.tsx
Outdated
Show resolved
Hide resolved
frontend/src/modules/WellLogViewer/view/SubsurfaceLogViewerWrapper.tsx
Outdated
Show resolved
Hide resolved
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.
Some remarks, otherwise good to go 👍
}); | ||
|
||
// TODO, do an offsett or something, so they dont always start on the same color? | ||
const colorSet = React.useRef<ColorSet>(new ColorSet(CURVE_COLOR_PALETTE)); |
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.
Could this be a constant outside this function?
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.
Sure, originally wanted each track to maintain their own internal color iteration, but it actually makes more sense that they all take from the same instance; that way, colors dont repeat as you add tracks and plots (unless you loop the entire palette, of course)
<Dropdown | ||
value={props.plot.type} | ||
options={PLOT_TYPE_OPTIONS} | ||
onChange={(v) => handlePlotChange({ type: v as TemplatePlotTypes })} |
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.
as TemplatePlotTypes
does not seem to be necessary.
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.
Yes. Also could get rid of the one used on "addPlot" aswell, just needed to update the AddItemButton
comp to pass the type
This PR is equivalent to #702; this is copy-branch to resolve some stuff that broke during some rebases
Branch dependencies
This uses the readout box and sortable list components introduced in other PRs, those should be reviewed and merged first
SortableList
component (+ sub components) and menu utility components #685(Partially) resolves issue #683
Adds a wrapper module for the Subsurface WellLogViewer component. The component renders out well-log data along a given wellbore's MD/TVD values. Note that this implementation is a simpler MVP, an is therefore not feature-complete; more features will be introduced in separate PRs.
Features:
well/log_curve_data
andwell/wellbore_log_curve_headers
API endpointFuture Work