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

remove schemeCategory20c in a backward-compatible way #1403

Closed
gordonwoodhull opened this issue Apr 11, 2018 · 17 comments
Closed

remove schemeCategory20c in a backward-compatible way #1403

gordonwoodhull opened this issue Apr 11, 2018 · 17 comments
Milestone

Comments

@gordonwoodhull
Copy link
Contributor

In #1394 I wrote:

There are good reasons why schemeCategory20c was removed. So we should switch probably to schemeCategory10c or a colorbrewer scheme e.g. as suggested in #831.

This means fixing a lot of tests, documenting the change pointing to the rationale in d3v5, and offering the workaround used here in the documentation. IMO we should not provide schemeCategory20c in the library because it is misleading.

@kum-deepak wrote:

Alternatively we can introduce a global switch to keep current behavior with a warning. In 3.1 we can make full switch.

I replied:

The color brewer colors are dramatically different but much more clear, so I agree we should probably do backward compatibility with a warning.

We don't currently have any global flags, but that's an interesting idea. Maybe it would be worth introducing a special facility under dc just for this, say dc.compatFlags, since we have so many things we want to fix while allowing backward compatibility.

In this case, there could be a flag, say brewerCategoricalColors, which defaults to false. The first time we need defaulted categorical colors, because the chart doesn't have any colors set, we check this flag and if it's still false, we use the copied colors from above and issue a warning that says we're moving to brewer and mentions schemeCategory10c, which is close to the old default but obviously has less colors.

If it's true, we use the color brewer colors with no warning.

This is just a suggestion; there are other ways to implement this. But in any case, we should not write to d3. src/d3v3-compat.js should go away before we release dc.js 3.0

@gordonwoodhull gordonwoodhull added this to the 3.0 milestone Apr 11, 2018
@kum-deepak
Copy link
Collaborator

Thanks for creating this :)

@kum-deepak
Copy link
Collaborator

Can you suggest which colors should we use as default. Planning to work on this now.

@gordonwoodhull
Copy link
Contributor Author

Tough choice! Please start with schemeSet1 and we'll see how it works. It is going to change our look dramatically!

@tttp in his proposed #831 noted that there may be difficulties with the default white label text and some background colors. I can't recall if this was a problem with the old color scheme.

Elsewhere I have implemented labels that switch from white to black depending on the luminosity of the background color. We could add that if we need it.

@kum-deepak
Copy link
Collaborator

Please see #1409

I have created a framework so that changing the pallet will not cause huge breakage in the specs.

@gordonwoodhull
Copy link
Contributor Author

@kum-deepak implemented a nice solution with a new global configurationdc.config.defaultColors which defaults to a copy of the old colors, deprecated with a warning.

Released in dc.js 3.0 alpha 10.

@tadiraman
Copy link

@gordonwoodhull i dont think 3.0 alpha 10 is working, actually the comment at the top of the file says dc 3.0.0-alpha.9. This change is breaking the cross filter charts. Is there any immediate solution to this?

@gordonwoodhull
Copy link
Contributor Author

Please try beta 1, which was just released today.

What do you mean, not working? If you want help you always need to describe the problem and how to reproduce it. In particular, what errors do you see in the browser console?

@tadiraman
Copy link

@gordonwoodhull
Here is the console log error.
You are using d3.schemeCategory20c, which has been removed in D3v5. See the explanation at https://github.com/d3/d3/blob/master/CHANGES.md#changes-in-d3-50. DC is using it for backward compatibility, however it will be changed in DCv3.1. You can change it by calling dc.config.defaultColors(newScheme). See https://github.com/d3/d3-scale-chromatic for some alternatives.

@gordonwoodhull
Copy link
Contributor Author

gordonwoodhull commented Apr 21, 2018

That's a warning, not an error. It tells you what you can do to make the warning go away.

If your charts are not displaying, there could be many reasons for that. You'd have to post a reproducible example, preferably on SO or the user group, in order for me to help you figure out what is going wrong.

@tadiraman
Copy link

OK. You are right! The actual error which caused breaking was the way d3.csv function working. I think some examples refer the old way d3.csv("filename", function(error, data){}); I did correct to d3.csv("filename").then(function (data) {});

@gordonwoodhull
Copy link
Contributor Author

Yes, that has changed with D3v5.

If you see an example we missed, please let us know. Aside from every fiddle and every SO answer now being wrong, I think we have updated everything in the repo and website.

@tadiraman
Copy link

OK. Thanks for the quick responses. I appreciate it!

@tttp
Copy link
Contributor

tttp commented Apr 23, 2018 via email

@gordonwoodhull
Copy link
Contributor Author

Agree, light/dark text classes are the way to go.

@badrinath389
Copy link

badrinath389 commented May 30, 2018

Warning on D3V5.
"d3": "5.1.0",
"dc": "3.0.0",
"crossfilter": "1.3.12",

In Angular5

import * as dc from 'dc';

## Getting below warning for this code dc.barChart('#variationDollarGoal')

dc.js:965 You are using d3.schemeCategory20c, which has been removed in D3v5. See the explanation at https://github.com/d3/d3/blob/master/CHANGES.md#changes-in-d3-50. DC is using it for backward compatibility, however it will be changed in DCv3.1. You can change it by calling dc.config.defaultColors(newScheme). See https://github.com/d3/d3-scale-chromatic for some alternatives.

Can you give me some suggestions? Thanks in advance

@kum-deepak
Copy link
Collaborator

@badrinath389 it is a warning, please check https://github.com/d3/d3-scale-chromatic for some alternatives. If you are confused, you can ignore the comment.

@gordonwoodhull
Copy link
Contributor Author

The warning is mostly there because the colors will change (probably to one of the colorbrewer schemes) in 3.1, so you need to take steps if you want to keep the old colors.

The old colors are arguably defective because they imply relationships which are not there - see the above links for details.

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

No branches or pull requests

5 participants