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

Color by legend is broken for numeric traits #926

Closed
huddlej opened this issue Mar 6, 2020 · 4 comments · Fixed by #927
Closed

Color by legend is broken for numeric traits #926

huddlej opened this issue Mar 6, 2020 · 4 comments · Fixed by #927
Labels
bug Something isn't working

Comments

@huddlej
Copy link
Contributor

huddlej commented Mar 6, 2020

Selecting a color by option that has numerical values produces an error that "something has gone wrong". For example, selecting "Age" in the novel coronavirus build (https://nextstrain.org/ncov?c=age) produces this output:

image

Using git bisect, I tracked this error down to the truncation of legend labels using the substring method.

I'm not familiar enough with React to implement a solution here, but it seems like we need to check the type of the label prior to applying substring.

@huddlej huddlej added the bug Something isn't working label Mar 6, 2020
@joverlee521
Copy link
Contributor

Thanks for tracking this down! I think we can throw String() around the label to convert everything to a string before calling substring

@tsibley
Copy link
Member

tsibley commented Mar 6, 2020

Yeah, this is a JavaScript type thing. Strings have a substring method, but Numbers don't. I expect a forced conversion to string is needed. There are many ways to format a Number, but label + "" might be enough.

Edit: or what Jover said!

huddlej added a commit that referenced this issue Mar 6, 2020
Fixes a bug where the tree legend could not be displayed for numeric color by
values because those values do not have a substring function.

Closes #926.
@huddlej
Copy link
Contributor Author

huddlej commented Mar 6, 2020

Ok, I actually was able to fix the issue with an explicit check for type (5f4a6d7), but if y'all prefer casting to string for everything, that's fine, too.

@tsibley
Copy link
Member

tsibley commented Mar 6, 2020

Thanks! I think that type check is fine. The label will be cast to a string anyway as part of the template interpolation, but I think it does make more sense not to apply character-length limits to a numeric value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants