ClinGen / clincoded

This GCI/VCI 1.0 platform has now been retired, and replaced with our new 2.0 platform:
https://github.com/ClinGen/gene-and-variant-curation-tools/issues
MIT License
25 stars 9 forks source link

Create Case-Control Curation page #1074

Closed selinad closed 8 years ago

selinad commented 8 years ago

Create a page for curating Case-Control information.

It will allow the following:

For now, we should use the following sections on the current Group page as a template for both Case Cohort and Control Cohort:

We will remove fields for the Group info section and relabel to Case Cohort or Control Cohort wherever appropriate.

Could be side-by-side or vertical....

There is a preliminary wireframe in Asana.

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw, @kilodalton, @kgliu0101, @mrmin123,

I have an instance spun up to give you a snapshot of what I have so far: https://1074-jz-case-control-page-ae33ea7-jzhen.demo.clinicalgenome.org

At the moment for this instance, I have only implemented most of the expected UIs. I have not wired up everything yet, including the logic to submit the form and/or display pre-existing form data. But I do wish you all can review the UIs and give me some feedback.

Note: To access the Case-Control page, go to Curation Central and click on the Case-Control bar in the Curation Palette.

kgliu0101 commented 8 years ago

@jimmyzhen Great one.

In control cohort Power, field "Enter numeric value of all Cases genotyped/sequenced" should be "... of all Controls ...". And, it looks to me that next field "Case Allele Frequency" should be "Control Allele Frequency", but not sure. Need to be confirmed by curators.

jimmyzhen commented 8 years ago

Thank you, @kgliu0101!

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw, @kilodalton,

Currently, we display the user-provided group label right above the Group panel. In the case of the Case-Control page which has 2 Group panels, how do we want this UI to behave?

image

selinad commented 8 years ago

HI @jimmyzhen - wow! I'm really sorry to be so behind with the instances. This is looking great!

I will have to spend more time looking through it and will do so today. Quick thing - I'm realizing we will want to have them name their Case-Control study, so we will need a label for that (so sorry as that is not in the specs). It should probably go at the very top.

Thank you for all of your hard work on this.

selinad commented 8 years ago

Hi @jimmyzhen - as mentioned in person, we should have a Case-Control section that spans both the Case Cohort and Control Cohort and allows a user to enter a label for the Case-Control.

As far as representing the labels, maybe we could do the following:

[Record icon] // Case-Control label (Case: label; Control: label) e.g. [Record icon] // Study 5 (Case: MYH7 group; Control: matched population)

wrightmw commented 8 years ago

@jimmyzhen This is looking great!

I agree with entering a label for 'CaseCohort-ControlCohort'... and this should be the title for the whole group

selinad commented 8 years ago

Hi @jimmyzhen

Such great work. Thank you. Here are my comments as I go through:

Front page styling

I like the Case Level and Case-Control gray images. One quick thing - they grab my eye more than the text in blue, though, and I miss the 2 bigger categories (Genetic Evidence and Experimental). Also, blue text often means a hyperlink - could we maybe make that text black and not italic? The font size seems good. Same comment on Case-Control page with blue text (not urgent!)

screen shot 2016-10-24 at 5 42 05 pm

Section for entering Case-Control label

We talked about this in person, but will write it down so it's recorded - have it span the 2 cohorts and have a place to enter label ( @kgliu0101 also knows) - see earlier comment for how labels displayed

Previous testing

@wrightmw - I'm realizing we probably don't need "Previous testing" for the Control group - do you agree? We can send this along for feedback from the group or give RG access to the instance.

Power

I think RG should look at the next instance and make sure all is correct.

wrightmw commented 8 years ago

I would change 'Experimental' to 'Experimental Evidence'

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw,

I have the following new instance for you to review the UI changes we discussed yesterday: https://1074-jz-case-control-page-aba822e-jzhen.demo.clinicalgenome.org

Please feel free to share with RG as well if needed. Thanks.

selinad commented 8 years ago

Great - thanks, @jimmyzhen !

selinad commented 8 years ago

@jimmyzhen just opened it up, but I already have to say great job! on the UI for the Record page (genetic evidence vs. experimental). @wrightmw - do you think a mouse icon would work for experimental?

selinad commented 8 years ago

@jimmyzhen also love the headers on the case-control page itself - really nice job.

I've sent it along to Raj - we need his feedback to make certain we spec'd all fields correctly. Will report back.

thx again - great work!

selinad commented 8 years ago

@wrightmw I just thought of one things for now - we could add "(Case)" or "(Control)" to each header on the Case-control page (except Disease/pheno one) - while not a different color, it would give quick reference since the page is long.....

wrightmw commented 8 years ago

@jimmyzhen Great work! I like it, a lot.

wrightmw commented 8 years ago

@selinad ... there are lots of other models used, other than mice.. how about a conical flask?

selinad commented 8 years ago

hehe. yep, as thinking a mouse was a fair catch-all. I'll take a look on font awesome - flask seems in vitro to me

wrightmw commented 8 years ago

Yeah... I was thinking of a conical flask as a generic 'experiment' icon... but could always use DNA?

jimmyzhen commented 8 years ago

Flask: http://fontawesome.io/icon/flask/

selinad commented 8 years ago

DNA might imply sequencing. I'd say a mouse or a cell - not sure where to find those icons...

ok - flask not bad!

wrightmw commented 8 years ago

@selinad Only one of the six experiment types is animal models and even these are often not in mouse. Whereas I think a flask is a generic icon for all experiments.

selinad commented 8 years ago

I think it's a question of what brings experimental type of evidence to mind. For me, it's a mouse for this type of evidence (could even be mouse cells), but I'm not opposed to a flask (even though none are done in a flask ;-))

jimmyzhen commented 8 years ago

How's this?

image

selinad commented 8 years ago

Hi @jimmyzhen - thx, it works! I still cast my vote for a mouse or an animal cell (this type of evidence is really functional evidence about the protein), but if not easily accessible, the flask gets the message out!

screen shot 2016-10-25 at 1 16 11 pm

wrightmw commented 8 years ago

@selinad I do think that with the page being long there should be something that shows you one side is case and the other control as you scroll down. I think it would be good if we added "(Case)" or "(Control)" to each header on the Case-control page.

jimmyzhen commented 8 years ago

There isn't a mouse icon readily available in Font Awesome, so we may need to look into other stock icons or create one ourselves (which I don't mind doing at a later time).

selinad commented 8 years ago

thx @jimmyzhen flask is fine for now :) We'll see if anyone objects or has better idea.

jimmyzhen commented 8 years ago

How's this styling to label the "Case" or "Control" sections?

image

wrightmw commented 8 years ago

@jimmyzhen It looks fine to me.

selinad commented 8 years ago

Oops - saw @wrightmw 's comment and realized I never sent:

Looks great! Super idea, @jimmyzhen! This could be a place where the color (orange) could be slightly different for case v control - wouldn't even mind that dark gray for control (color implies case, I think)...

wrightmw commented 8 years ago

@selinad well, as you know, I am in favour of a colour differentiator between case and control... so I support your suggestion.

selinad commented 8 years ago

👍 I suggested with you in mind @wrightmw !

jimmyzhen commented 8 years ago

How's this?

image

selinad commented 8 years ago

Super sweet. :-)

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw,

What changes would you want to apply to the intermediate page?

image

selinad commented 8 years ago

Hi @jimmyzhen Here are comments from Raj + 1 from me:

I have 2 follow-up questions I need to ask him.

selinad commented 8 years ago

Re the intermediate page: Probands counted in Case-control would not be counted in Case (Family/Individual), so I am pretty certain we don't need an intermediate page ( @wrightmw do you agree?).

wrightmw commented 8 years ago

@selinad I agree. No intermediate page. We don't want double counting of probands.

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw,

Here is the new instance for you to review the Case-Control page: https://1074-jz-case-control-page-c25c74b-jzhen.demo.clinicalgenome.org

Known issues at the moment: 1) After saving the Case-Control page, returning to the Curation-Central page appears to be working as expected. However, returning to the Dashboard page would cause errors in which the histories are not loading as expected. 2) The Case-Control statistics input fields currently expect numeric values. Errors may occur if no values or string values are entered. Need to add validation for those fields.

selinad commented 8 years ago

Hi @jimmyzhen a very quick look looks great!

only a couple of things:

Super job! I'll pound on it a bit more (in a bit)

selinad commented 8 years ago

Hi @jimmyzhen - looking at it again, I think "Numeric value of ..." should be changed to "Number of..." in all locations (e.g. Number of Cases with variants in the gene of question")

Most recent changes from Raj:

Action items based on above:

selinad commented 8 years ago

Was not able to submit (spinning wheel) - I had filled out most of Case, but not cohort or evaluation section:

screen shot 2016-10-27 at 10 50 30 pm
wrightmw commented 8 years ago

@selinad I just had the same experience. Unable to submit... had spinning wheel.

selinad commented 8 years ago

@jimmyzhen had this same problem when I started again.

selinad commented 8 years ago

oops - thx @wrightmw . Do you know what caused it? I'm not exactly sure what I did.

wrightmw commented 8 years ago

I just went through and filled in most fields and when I went to save I got a spinning wheel.

jimmyzhen commented 8 years ago

The spinning wheel is most likely caused by certain missing form validations. I will look into it.

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw,

I have addressed the changes you described above, as well as the bugs you experienced in the previous instance (e.g. spinning wheel, failed deletion, dashboard history). I will spin up a new instance shortly for you to review again.

jimmyzhen commented 8 years ago

Hi @selinad, @wrightmw,

Below is the new instance for you to review: https://1074-jz-case-control-page-045a7f6-jzhen.demo.clinicalgenome.org

Sorry for taking this long to have it ready :(

In addition to include the addition of a Case-Control to the history on the Dashboard, I also needed to include the modification and deletion of a Case-Control to the history on the Dashboard (as I realized later on).

Furthermore, upon adding a Case-Control to the history, I realized that the links in the histories would take the user to the Case-Control view page. Since suppressing the links in the histories would seem to defeat the purpose of the having the histories, so I went ahead and created the Case-Control view page. It is not pretty, but it works. As a result, the "View" link for the Case-Control in the CurationPalette is now reinstated.

Feel free to give me your feedback after reviewing the instance. Thanks!

selinad commented 8 years ago

Wow! Thanks @jimmyzhen - taking a look now. btw, REVEL back up on this instance (thx for sending email @wrightmw )