The 'height' and 'width' keys are ignored in barchart.js while drawing the chart. The attached patch should fix it.

Comments

jorijn’s picture

StatusFileSize
new857 bytes
asherry’s picture

I think this patch makes height and width required then, right?

asherry’s picture

Status: Active » Postponed (maintainer needs more info)
alansaviolobo’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new4.82 KB

have added two configuration to the style plugin - width & height.

these are optional and if not entered, the hard coded dimensions are considered.

anybody’s picture

Status: Needs review » Needs work

Well 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.

alansaviolobo’s picture

@Anybody, neither patch changes anything in the d3 library. the patch touches files in the views implementation(php/js) for the specific chart type.

asherry’s picture

Status: Needs work » Postponed
Related issues: +#2279815: Improve on views integration

Most 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.

anybody’s picture

@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?

anybody’s picture

Status: Postponed » Needs work
guignonv’s picture

Status: Needs work » Closed (outdated)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.