-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat - 194: ROI class based color settings #227
feat - 194: ROI class based color settings #227
Conversation
…ingDataCommons/slim into feat-194-sr-class-based-color-settings
Visit the preview URL for this PR (updated for commit c4213c3): https://idc-external-006--idc-slim-preview-bhe6cro3.web.app (expires Thu, 22 Aug 2024 17:05:48 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307 |
Looks great testing with https://healthcare.googleapis.com/v1/projects/idc-dev-etl/locations/us-central1/datasets/idc/dicomStores/annotated_rms-original-and-annotations-wc-temp-20240122/dicomWeb (specifically, RMS2455 is a good example). Few suggestions: |
Isn't it showing on your end? For me it shows Or do you mean something else?
I'll work on this. |
Indeed, it does show! I guess I was confused by the colors. Why is category black and type green-blue? I think all should be black |
79cda95
to
50f2475
Compare
@fedorov you can check the changes you required here |
@pedrokohler this is perfect, great work! Good to go from my perspective. |
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.
First iteration, few questions!
@pedrokohler can you please use the link below and check if you can replicate this error? |
a6474dc
to
753162e
Compare
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.
Almost there! I notifed the following issue:
- Open the study
- Don't activate any ROI
- Go to the categories section but don't select any
- Change the color of one of the categories without activating
- The annotation is not visible but the category check is not checked
3bbbc61
to
125122e
Compare
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.
LGTM
Adding the ability to edit ROI colors based on their categories and types. Solves #194.