feat(viewer.js): Add bidirectional and ellipse annotations #72
Conversation
igoroctaviano
commented
Nov 25, 2021
igoroctaviano
commented
- Adds Ellipse and Bidirectional annotations
- Adds optical path fix / img type changes by @wayfarer3130
|
@hackermd I'm creating this PR as a draft because there's probably a ton of changes necessary to get this one merged. There's a new presentation state property in the ROI that you can ignore. It was used to store coordinates of the textbox. |
There was a problem hiding this comment.
@igoroctaviano this looks really great and the functionality would be very useful to have in the library. I didn't yet delve into the implementations details, but only glanced over the changes to the public API and the coding style.
Some initial comments:
- Please format the code according to our coding style guidelines based on standard. You can use
npm run lintandnpm run fmt. - We will need to find a different way of change the style without passing OpenLayers objects via the public API of the viewer.
I will take a closer look at the implementation of the markups and conduct a more comprehensive review next week.
|
|
||
| import _MarkupManager from './markups/_MarkupManager' | ||
| import _MarkupManager from "./markups/_MarkupManager"; |
There was a problem hiding this comment.
Could you please revert these stylistic changes? They complicate the code review and are not in compliance with the coding style of the library (see Coding Style section in CONTRIBUTING).
| new dcmjs.sr.coding.CodedConcept({ | ||
| meaning: "Long Axis", | ||
| value: "G-A185", | ||
| schemeDesignator: "SRT", | ||
| }), | ||
| new dcmjs.sr.coding.CodedConcept({ | ||
| meaning: "Short Axis", | ||
| value: "G-A186", | ||
| schemeDesignator: "SRT", |
There was a problem hiding this comment.
Please use SCT rather than SRT codes.
| meaning: "AREA", | ||
| value: "G-D7FE", | ||
| schemeDesignator: "SRT", |
There was a problem hiding this comment.
Please use SCT rather than SRT codes.
|
|
||
| const addOrUpdateMeasurement = (feature, measurement) => { |
There was a problem hiding this comment.
This should probably be a private function, too.
| const addOrUpdateMeasurement = (feature, measurement) => { | |
| const _addOrUpdateMeasurement = (feature, measurement) => { |
| * @param {number[]} styleOptions.fill.color - RGBA color of the body | ||
| * @param {object} styleOptions.image - Style options for image | ||
| */ | ||
| setFeatureStyle (feature, styleOptions, optSilent = false) { |
There was a problem hiding this comment.
The public API of the viewer should not expose any of the underlying OpenLayers types. We will need to be abstract this.
igoroctaviano
changed the title
|
