Skip to content
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

Design #713

Merged
merged 10 commits into from
Jun 12, 2015
Merged

Design #713

merged 10 commits into from
Jun 12, 2015

Conversation

jaclynperrone
Copy link
Contributor

This is the first pass at the redesign for the Runs page (in addition to touch-ups on the VCF table and tooltips on the Examine page.)

While this is still a work in progress, I thought it would be good to get it out in the wild to start collecting feedback. The next PR will be more focused on the Examine page and VCF comparison.

Known issue: The svg's in the buttons are black, when they should be blue to match the styling. Going to address this issue in the next PR.

Review on Reviewable

@ihodes
Copy link
Member

ihodes commented Jun 10, 2015

Awesome! It's exciting to get some new design in here. I am particularly a fan of moving the latest comments feed to the side of the runs page. The improved visual differentiation between projects is also quite nice :)

A couple high-level comments:

  1. This is moving up to lower-contrast print with the darker, brown, backgrounds. This is a personal preference leaking through in a PR, but with that in mind I lean towards less color when functionality is important, reading the text is the primary interaction a user has with Cycledash; we should make it as easy as possible.
  2. I like the new colors for the buttons! Do you think it could confusing for buttons that are usually green/blue (positive UI colors) to be reddish (negative/reject colors)? Is there a particular reason behind the change?
  3. The change of some buttons (e.g. "download VCF") to the light blue color could also be a contrast issue. Is the issue that we need more attention brought to those buttons?

Looks good overall, and thanks for working on the design!

@jaclynperrone
Copy link
Contributor Author

Thanks for looking it over! Here are some thoughts:

  1. Was hoping to make CycleDash look different from the other tools a researcher might have open at any given point. Thought it would be nice to set it apart from the sea of white backgrounds that they're usually traversing. This also makes it easier for them to navigate to CycleDash as they hop around different tools.

The warmer tones are also a little easier on the eyes for when you are staring at rows and rows of data. I think there are some places (like table headers, labels) where it's just straight-up hard to read, so I'll look into that. I'm into pushing to an extreme and letting it marinate for a bit. It's interesting to see what works and what doesn't over time.

  1. So! Yes. I thought about this, too. I wanted to pull in the blue & fuchsia Mt. Sinai colors to give a little nod to the mothership. The fuchsia buttons signify form submissions, while the blue buttons/links are...well links! It's good to incorporate color as function, for it helps guide the user and reinforces the purpose of the buttons.

I tried switching the colors (blue for the button, fuchsia for the links), but it didn't translate as well. I'd like to let this sit for a bit if that's cool. I'm down to keep tweaking that fuchsia, maybe make it pop less. Also, if you have other colors you'd like to try out, go for it!

  1. To go with the last point, I'd like to let it sit for a bit. I was going back and forth with whether the download button should be fuchsia or blue (based on the above logic). It's not a link, so...it looks like it needs to change to fuchsia.

@hammer
Copy link
Member

hammer commented Jun 10, 2015

Can we have a staging server for Cycledash where we can deploy branches like these? I'd like to be able to see the new design live.

@hammer
Copy link
Member

hammer commented Jun 10, 2015

And yes I know we have screenshots--which are very useful!--but I'd love to interact with the changes as well.

@hammer
Copy link
Member

hammer commented Jun 10, 2015

I wanted to pull in the blue & fuchsia Mt. Sinai colors to give a little nod to the mothership

👍

@hammer
Copy link
Member

hammer commented Jun 10, 2015

I like how the new background color helps segment the product into functional sections. It's a bit dark on the screenshots for my taste, though.

And I can't wait for #650--the grayed out box for active comments coupled with the white box for previous comments is killing me.

@jaclynperrone
Copy link
Contributor Author

Can we have a staging server for Cycledash where we can deploy branches like these? I'd like to be able to see the new design live.

Maybe for the time being you can checkout the branch and run it locally?

@ihodes
Copy link
Member

ihodes commented Jun 11, 2015

I like the change in color for logically separating sections, but I do think it's harder to read text on that dark brown background. For text-heavy areas (the rows in the examine table, the about page, comments page, etc.) in particular, something closer to black on white seems ideal without sacrificing the goal of separation of sections.

I also like the nod to the mothership. Maybe we can restrict those colors to non-functional sections of the UI, though? A red save button and a blue cancel button is not something anyone would expect. I'm fine with moving forward with this PR, though, so this isn't a blocker. I'm just putting my vote out that functionality and ease of use should come before MSSM colors :) On that note, the MSSM colors are fairly different from the ones specified in the CSS; it is likely me being slow, but I didn't make the connection between the color scheme and Mount Sinai colors.

@jaclynperrone
Copy link
Contributor Author

I'll keep tweaking the colors. I would like the colors to help reinforce functionality, so will keep working on different options. I assure you these things will keep changing :). I'm cool with holding off on this pull request until we are in a happy place. So keep 'em comin'.

@ihodes
Copy link
Member

ihodes commented Jun 11, 2015

I'm definitely not suggesting removing colors! I do think a white (or near-white) background in text-heavy areas is a pretty solid choice that should have good justification for before changing.

@jaclynperrone
Copy link
Contributor Author

I hear ya.

@jaclynperrone
Copy link
Contributor Author

Alright! Revised version is up! Perhaps the world isn't ready for fuchsia 😉 It's also impossible to spell correctly...so there's that.

@hammer
Copy link
Member

hammer commented Jun 12, 2015

Maybe for the time being you can checkout the branch and run it locally?

For sure. Would be really nice though to have a continuous integration server pick up PRs and automatically deploy them to AWS and stick the URL for the deployment into a comment in GitHub. Just sayin'...

@jaclynperrone
Copy link
Contributor Author

That would be awesome!

@@ -79,6 +102,38 @@ section#api-docs > section {
/*------[ Table Styling ]------*/
.table {
font-size: 0.875em; /* 14/16 */
color: $color-text;;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra semicolon

@ihodes
Copy link
Member

ihodes commented Jun 12, 2015

Awesome, this looks good. I know it's a WIP, but getting even lighter/putting text (on e.g. /about and /comments) in a container like you're doing for projects might be nice (another day, another PR). This is great, though! 1 minor nit in a comment, and then merge away!

@jaclynperrone
Copy link
Contributor Author

Eagle eyes! Good catch on the semicolon! Thanks for giving it all a once over. I'll hit that shiny green button when the tests pass! 🎊

And I like the suggestion about the containers. Will definitely play around with that!

jaclynperrone added a commit that referenced this pull request Jun 12, 2015
@jaclynperrone jaclynperrone merged commit 1908b6f into master Jun 12, 2015
@danvk danvk deleted the design branch June 22, 2015 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants