Skip to content

DM-7965 make consistent highlight and selection colors #258

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

Merged
merged 2 commits into from
Dec 17, 2016
Merged

Conversation

tgoldina
Copy link
Contributor

This ticket is for making highlight and selection colors consistent in tri-view. (Highlight matching orange image border color, selection is yellow.)

@loitly
Copy link
Contributor

loitly commented Dec 17, 2016

Changes are fine. It works as expected.
However, the highlighted(orange) and selected(yellow) dots in plot look almost identical. Also, when there is only 2 images, I can't tell which one is highlighted. The blue border is just as strong as the orange one.
Of course, this is only my opinion. It should not stop you from merging.

@ejoliet
Copy link
Contributor

ejoliet commented Dec 17, 2016

Color seems fine. But I was randomly checking and the tooltip in histograms is broken, an error log message says:

Histogram.jsx:376` Uncaught TypeError: Cannot read property 'name' of undefined
at Object.formatter (Histogram.jsx:376)
at a.Tooltip.refresh (highcharts.js:188)
at a.Pointer.runPointActions (highcharts.js:198)
at a.Pointer.onContainerMouseMove (highcharts.js:206)
at HTMLDivElement.c.onmousemove (highcharts.js:208)

@ejoliet
Copy link
Contributor

ejoliet commented Dec 17, 2016

Looks that the problem is not part of the branch, but was introduced before (building 'dev' shows the same problem (and others)
A hint from @tgoldina : a change from "react-highcharts": "10.0.0", to "react-highcharts": "11.0.0" in package.json.

…it: true is causing tooltip to be shared, perhaps a bug in Highcharts. Anyway it's not needed. Added a way to handle shared tooltips, commented out split.
@tgoldina tgoldina merged commit 7fb7394 into dev Dec 17, 2016
@tgoldina
Copy link
Contributor Author

Emmanuel, thank you for finding the histogram tooltip issue. I think, it was a side effect of migrating to Highcharts 5. Anyway, I have fixed this issue in this pull request.

I decided to merge it, so that people have something to look at: the highlighted color is "orangy" in all 3 views, and selected color is "yellowish" in both image and plot.

The colors are easy to adjust as long as we decide what they should be. However we should be aware that the same color looks different on different background. I made table highlight lighter shade of orange, and adjusted the saturation of xy plot highlight to make it different enough from the yellow of the selected points. I realize that it might not be perfect, given that color perception is different from person to person. Still, it's more consistent than it was. :)

@tgoldina tgoldina deleted the dm-7965_colors branch December 17, 2016 04:51
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