monarch-initiative / monarch-legacy

Monarch web application and API
BSD 3-Clause "New" or "Revised" License
42 stars 37 forks source link

Analyze Phenotype: workflow redesign #1250

Open yuanzhou opened 8 years ago

yuanzhou commented 8 years ago

Based on @harryhoch's mockups, I started the redesign. The first thing I did was to clean up the page and related code. I also rephrased the section titles as you can see from the second image. In section of step 2, instead of organizing the search and compare side by side, I made them vertically aligned, so the UI looks cleaner. I've also removed the reset button since it doesn't work well. And made the submit button lots bigger.

OLD version:

old

NEW version:

new

@harryhoch @jmcmurry please add comments and I'll keep refining. I created this issue to encourage discussions during this redesign. This is just the starting point.

yuanzhou commented 8 years ago

Made some wording changes. Are "Select Comparables" and "Selected Comparables" appropriate?

1

And added the selected inputs on the result page:

2

harryhoch commented 8 years ago

fine. Are you going to make the selected phenotypes removable?

yuanzhou commented 8 years ago

@harryhoch I'm going to play with that. Every time we remove one phenotype, the whole page will need to be reloaded to refresh Phenogrid, which also means we'll have to wait for the owlsim querying. Not sure if there's a more efficient way of doing this.

harryhoch commented 8 years ago

@yuanzhou, not necessarily. Perhaps include a "refresh" button which would only be enabled once someone changes the phenotypes. No update of phenogrid until that button is pressed...

yuanzhou commented 8 years ago

@harryhoch One thing I realized is the selected phenotype IDs are in the URL, for example:

http://localhost:8484/analyze/phenotypes/?input_items=HP%3A0001937+HP%3A0012594+MP%3A0009235&mode=search&target_species=all&limit=100

There are 3 phenotypes, and when I make the phenotypes removable, we'll also need to update the URL to refresh the result, including Phenogrid.

harryhoch commented 8 years ago

would it help to remove the phenotypes from the URL and to go to a PUT instead of a GET? Might require some monkeying on the middleware, but shouldn't be too bad.

jmcmurry commented 8 years ago

My feeling is that both the original and modified versions of this interface plunge the user in a bit too fast into 'advanced' mode. I would love to see less emphasis on the homologs/species and more on dynamic results and faceting as previously discussed.

analyzephenotypes_redesign_general_2016-06-03-01

harryhoch commented 8 years ago

I don't disagree. Should we rethink efforts/priorities?

kshefchek commented 8 years ago

In response to https://github.com/monarch-initiative/monarch-app/issues/1250#issuecomment-223682134, I think it makes sense to have this page be completely client driven, the server just provides the table view, and that can easily be moved to the browser - and probably needs some work anyway.

We shouldn't remove the URL params entirely, because this is still used from the annotate text page to send a list of phenotypes to the analyze page. But once on the page it should be single page app-like.

harryhoch commented 8 years ago

@kshefchek , it can work both ways, right? we can enter the page with a get and then change content from there based on user interaction...

kshefchek commented 8 years ago

Yep, we're on the same page!

jmcmurry commented 8 years ago

@yuanzhou what are all the contents of this dropdown menu currently?

screen shot 2016-06-07 at 2 42 03 pm
jmcmurry commented 8 years ago

It is my opinion that the two most crucial things to fix are: 1) Combine inputs / results on one (dynamic) page 2) Make genes, models, diseases discrete sets of results (each of these solves a different use case). Conflating them is more confusing than needs to be.

Both of these objectives are steps the path to the more comprehensive redesign originally proposed.

I've taken a stab at how we might accomplish the above two objectives with the existing widgets, but would like to know whether objective 2 is problematic from the backend standpoint and phenogrid more generally. I would like if possible not to have a refresh button for phenogrid, but we can certainly revisit if it is too intense. The table parts should be easily made dynamic. Thoughts?

analyzephenotypes_redesign_general_2016-06-07b-01

How do people feel about this as a first pass? Is it doable before July? @harryhoch @cmungall

harryhoch commented 8 years ago

Julie: A few comments.

  1. Goals one and two sound reasonable.
  2. Are you suggesting populating the discrete models, genes, and diseases based on search terms? Seems reasonable to me, but I must defer to @cmungall , @yuanzhou , @cborromeo , and @kshefchek for feasibility.
  3. Not sure what you mean by "not having a refresh button for phenogrid." At the moment, I think we can't have an automatic refresh. Simsearch calls take a few seconds to return values, so we don't want the delay triggering unless users explicitly request.

As far as doable by July, I'm skeptical. Possible, but I don't like rushing these things. I'm also skeptical that this can be divvied up amongst multiple folks. @yuanzhou, what do you think?

yuanzhou commented 8 years ago

@jmcmurry in my branch, the current drop down contains

We disabled the "Caenorhabditis elegans (genes)"

yuanzhou commented 8 years ago

@harryhoch I agree with you regarding the automatic Phenogrid refresh. Sometimes simsearch calls take more than several seconds.

I'm not positive that I can get this done by July considering I'm also tied to several other projects. I'll try my best. And a clear design goal will definitely be very helpful to avoid miscommunication and guess work.

yuanzhou commented 8 years ago

@jmcmurry regarding your second point:

2) Make genes, models, diseases discrete sets of results (each of these solves a different use case). Conflating them is more confusing than needs to be.

Are you suggesting to have a rendered Phenorgrid under the data table at each discrete section?

As @harryhoch mentioned, I'll probably need some input from @kshefchek

jmcmurry commented 8 years ago

Are you suggesting to have a rendered Phenorgrid under the data table at each discrete section?

Yes, that's my proposal but not all three phenogrids in one page at one time. Just one phenogrid, corresponding to whichever tab the user clicks (Genes, Diseases, Models). A side-benefit of this decoupled approach is that there would be fewer species to render. (Diseases would be human only, models would be mostly not human)

cmungall commented 8 years ago

Sometimes simsearch calls take more than several seconds

We need to reduce this. I suspect the owlsim call itself is fast, but the app wrapper is doing other stuff

jmcmurry commented 8 years ago

@cmungall what is your preference? Should we: A) Aim to improve response times before July OR B) Focus on UI improvements and do a "refresh" button as a short-term fix

Without dynamic results, I'm not convinced there's much to be gained by having inputs/outputs on the same tab. Regardless of where we go with the interface, performance would need to be improved anyway. By reducing the scope of any one call to a particular type of result (Gene, Disease, or Model), wouldn't performance be better anyway (since you're reducing the search space)?

cmungall commented 8 years ago

There is no model search in the current owlsim2 instance. Only human d2p and model organism g2p associations are loaded.

we need more data before making a final decision. Can we simulate the calls required in a typical session in a script and do some benchmarks using that?

harryhoch commented 8 years ago

@cmungall, I might be able to do something quick and dirty tonight. will see what I can come up with.

harryhoch commented 8 years ago

@cmungall, just did some rough checks. Saw response times to POSTS to https://monarchinitiative.org/simsearch/phenotype ranging from .001s to .925 sec. We'd like to see < 200ms for perception of instantaneous update.

jmcmurry commented 8 years ago

Related to https://github.com/monarch-initiative/monarch-app/issues/1238; please check and close if appropriate. Thanks

harryhoch commented 8 years ago

ok. tried further running against local monarch-app with no cache on results . I'm seeing calls to https://monarchinitiative.org/owlsim/searchByAttributeSet going from .1 sec to 3.2 sec. @cmungall, these are the calls that go straight to OWLSim, right?

jmcmurry commented 8 years ago

So having spoken, we decided we would aim to 1) Put inputs, phenogrid, and table on one (visual) page 2) Provide a refresh button for phenogrid and table Ultimately, it would be good to get away from this, but whatever we do, let's not do as MBTA does: They not only require a click to update but they continue to display data brazenly without even indicating it no longer corresponds to the (revised) user inputs. This too is strange: their REST API has "RedisplayTime=Redisplay+Time" in it. As if stale data would be worth having.

screen shot 2016-06-09 at 11 48 51 am

We could either: A) Have some kind of greying out of the table and grid to flag the fact that it is necessary to click to update. B) Change the appearance of the input UI so that it clearly has two modes, active and inactive. An example of a site that I think does this well is Kayak.

screen shot 2016-06-09 at 12 11 52 pm screen shot 2016-06-09 at 12 11 42 pm

Thoughts?

harryhoch commented 8 years ago

also, @jmcmurry suggests hiding the advanced options in a accordion style display so they aren't seen unless needed.

yuanzhou commented 8 years ago

Any strong opinion that we should keep the "Orthologs" and "Paralogs" buttons? I made progress with the one page design, I'm also trying to make the comparables section as short as possible. And since we are having that "discrete sets of result" idea in mind, it might be helpful to focus fewer input aspects as a new starting point.

kshefchek commented 8 years ago

It's an important use case, we need to either leave it in this widget or make the option available elsewhere.

jmcmurry commented 8 years ago

Agreed we need to keep them. Perhaps hiding within an "advanced" button though?

harryhoch commented 8 years ago

+1

yuanzhou commented 8 years ago

So far I've got the one page working.

1

The orthologos and paralogs buttons will show up only when the genes option is selected. capture

Once the submit button is clicked, the phenogrid and data table will show below the button. Whenever there's a new phenotype is added or one gets removed from the selected list, the phenogrid and data bale will fade out. This encourages the users to reclick the submit button to get new results.

jmcmurry commented 8 years ago

The orthologos and paralogs buttons will show up only when the genes option is selected.

Thanks Zhou. This is a great start; I also like the following you added:

I have a few suggestions in rough order of importance:

screen shot 2016-06-15 at 1 38 03 pm
harryhoch commented 8 years ago

@yuanzhou, I agree - this is a great start.

@jmcmurry, I like your changes as well. Zhou, do you think you can add these changes in? Thanks.

yuanzhou commented 8 years ago

@jmcmurry Thanks for the great suggestions, one thing to clarify:

Make the text on the buttons just say "submit" and in larger font

It says "Analyze" in your mockup, which one do you prefer?

@harryhoch I'm working on those changes. And regarding the Step 2, I also think it might be better to display those species in checkboxes instead of dropdown menu. Checkbox allows users to select any combinations they want to analyze against. Right now the dropdown only gives options of single species or all. In some use cases, e.g., user may want only see mouse or human. Even though you can enable/disable species via the "Options" button of Phenogrid, but this won't update the data table.

harryhoch commented 8 years ago

@yuanzhou Ok. If space is at a premium, you can put checkboxes in a dropown.

jmcmurry commented 8 years ago

Thanks 1) As for submit vs analyze I don't feel strongly. Harry? Others? 2) As for checkboxes, that is a good idea. We can probably spare a little vertical space for a few rows. Expand-collapse dialog with checkboxes could be fine too. Hard to know until we look at it.

harryhoch commented 8 years ago

I'll go for "analyze", but not for any strong reason

cmungall commented 8 years ago

1) As for submit vs analyze I don't feel strongly. Harry? Others?

Search?

mellybelly commented 8 years ago

Well the title of the page is "Monarch Phenotype Profile Analysis", so maybe "Analyze"?

jmcmurry commented 8 years ago

Sounds good. I don't love 'search' as that sounds a bit pedestrian. OwlSim is anything but :) Go, OwlSim, Go!

yuanzhou commented 8 years ago

After the demo this afternoon, @jmcmurry suggested to add back the gene helper text as well as link the gene ID section to https://github.com/monarch-initiative/dipper#identifiers.

capture2

I've also made the results grayed out on adding/removing new phenotypes, and changed the submit button color and text to highlight the change.

capture

harryhoch commented 8 years ago

Nice, @yuanzhou , thanks