-
Notifications
You must be signed in to change notification settings - Fork 290
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
Custom colors for heatmap #253
Conversation
For 3.6.0 Release
Remove Global declaration
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.
Great work, just one little nitpick :)
$(container).children().each(function (index,colorDiv) { | ||
rs.push(colorDiv.querySelector('input').value) | ||
}); | ||
return rs.reverse(); |
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.
I'll invite @nanli-emory to comment on this too, but I think it makes more sense to show the lower value heatmap bins first.
return rs.reverse(); | |
return rs; |
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.
Agreed, i used the reverse order because earlier legends were in that order. I have a feeling that we will need to update legend icons code also. I'll check and update accordingly.
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.
Also, @birm @nanli-emory what should be maximum limit on number of intervals. As of now i have made min 2 and max 5 intervals.
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.
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.
max 10 intervals
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.
The logic for gradient should go from lighter colors for less values to more darker colors for higher values, or from blue to red?
Above screen shot has gradient from white to blue for lower to higher values. Of course user can change color of each interval. If this isn't appropriate, @nanli-emory please suggest the 10 colors for which gradient should be formed.
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.
waiting for your reply @nanli-emory.
Corrected validation for legend colors.
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.
Ah, another nitpick! Otherwise, this looks good to me!
Co-Authored-By: Ryan Birmingham <birm@rbirm.us>
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!
Added options to select color for heat maps in both gradient and binal mode.
closes camicroscope/Distro#93