Closed (outdated)
Project:
d3.js
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
25 Nov 2013 at 07:18 UTC
Updated:
17 Apr 2026 at 19:10 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
jorijn commentedComment #2
asherry commentedI think this patch makes height and width required then, right?
Comment #3
asherry commentedComment #4
alansaviolobo commentedhave added two configuration to the style plugin - width & height.
these are optional and if not entered, the hard coded dimensions are considered.
Comment #5
anybodyWell I've just had a look at the patch and I think the problem addressed in this issue truely exists.
Anyway I think it's bad style to change a library. The library should offer a parameter that is being set by the Drupal module on instantiation. A change to the library should never be required.
Comment #6
alansaviolobo commented@Anybody, neither patch changes anything in the d3 library. the patch touches files in the views implementation(php/js) for the specific chart type.
Comment #7
asherry commentedMost of this patch will get committed, but it is going to depend on #2279815: Improve on views integration. That views integration patch will add a lot of functionality for defining variables that are needed for a library in the .info file.
I agree there shouldn't be direct changes to a library, what users should do is either clone out the library or create some settings / field / meta data in their library info files that will map to the library.
If you can, it'd be great for you to check out the improve-views branch and try out some visualizations there. Once that is committed I'll commit this too.
Comment #8
anybody@alansaviolobo: You are right. I was confused by the folder naming "library" within the d3 module which contains the initialization and not the d3 library itself as it seems.
Anyway I think the given patch in #4 does not fit the Drupal patch conventions, at least it fails for me agains the latest .dev version. But that may also be my problem. So lets have some more oppinions.
@asherry: You are right and I think #2279815: Improve on views integration is very important and useful in combination with this issue. What about a new branch for that to make the improvements more independent and easier and earlier to test?
Comment #9
anybodyComment #10
guignonv