qiime / qiime_web_app

QIIME, EMP and American Gut Web Application
www.microbio.me/qiime
9 stars 10 forks source link

Issue 46 #646

Closed adamrp closed 10 years ago

adamrp commented 10 years ago

Right now, I have it so that the new study id dropdown appears for all users, although I can see an argument for having it appear only if the logged-in user is an admin. Thoughts @teravest @ElDeveloper ?

teravest commented 10 years ago

:+1: I like this. I like this a lot. I'm not sure if users are going to know the study id. Do users know the study ids?
Also does this work for emp? We still have no way to test emp cause it redirects to www.microbio.me/emp when you login....

adamrp commented 10 years ago

it should work for EMP because they are literally the same pages, but you're right that testing is an issue. I thought we fixed, or at least had an idea of how to fix, that redirect?

The study ID of the selected study is displayed in the upper right corner for regular users as well as admins, so they might know their study id, although the feature is definitely more useful for people with hundreds of studies in that dropdown (e.g., us admins)

ElDeveloper commented 10 years ago

FYI, just deployed this to webdev, taking a look at it now.

adamrp commented 10 years ago

Awesome, thanks!

On Tue, Mar 18, 2014 at 3:13 PM, Yoshiki Vázquez Baeza < notifications@github.com> wrote:

FYI, just deployed this to webdev, taking a look at it now.

Reply to this email directly or view it on GitHubhttps://github.com/qiime/qiime_web_app/pull/646#issuecomment-37989201 .

ElDeveloper commented 10 years ago

Just took a look at it, great work @adamrp! The only thing that I noticed is that the bullet point seems to be a bit off:

screen shot 2014-03-18 at 3 20 51 pm

I would change Or by Study ID to be Study ID and I would also change Select an Existing Study by Name to be Project Name. If you guys agree with these changes, then the easiest might be to list these two drop downs under a new title called "Select a Study".

This looks great @adamrp.

adamrp commented 10 years ago

Fixed this up -- looks much better now, thanks @ElDeveloper!

adamrp commented 10 years ago

This is now on webdev, in case you want to have a look before merging (assuming no further comments)

ElDeveloper commented 10 years ago

This is great, thanks for making those changes!! :+1: