I've been playing with the new v4 version with custom charts for a project, and I found that #raw_options array is marged in a way that options cannot be overriden. For exemple, Im trying to pass 'isStacked': true to Google chart.

      '#type' => 'chart',
      '#chart_type' => 'bar',
      '#raw_options' => [
        'options' => [
          'isStacked' => TRUE,
        ],
      ],

I think the problem is in the order of the arguments to NestedArray::mergeDeepArray():

    // Merge in chart raw options.
    if (!empty($element['#raw_options'])) {
      $chart_definition = NestedArray::mergeDeepArray([
        $element['#raw_options'],
        $chart_definition,
      ]);
    }

If I change the order like the following, then it works:

    // Merge in chart raw options.
    if (!empty($element['#raw_options'])) {
      $chart_definition = NestedArray::mergeDeepArray([
        $chart_definition,
        $element['#raw_options'],
      ]);
    }

This affects Google and Chartjs, at least.

CommentFileSizeAuthor
#7 raw-options-3156731-7.patch1.37 KBGuillaumeDuveau
#4 interdiff_3_4.txt5.75 KBnikathone
#4 3156731-4.patch15.26 KBnikathone
#3 3156731-raw_options-does-not-allow-override-3.patch8.45 KBandileco

Issue fork charts-3156731

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markus_petrux created an issue. See original summary.

andileco’s picture

andileco CreditAttribution: andileco at JSI Research & Training Institute, Inc. (JSI) for JSI Research & Training Institute, Inc. (JSI) commented

Thanks for finding this, @markus_petrux! Will get it fixed soon.

andileco’s picture

andileco CreditAttribution: andileco at JSI Research & Training Institute, Inc. (JSI) for JSI Research & Training Institute, Inc. (JSI) commented
Status: Active » Needs review
FileSize
3156731-raw_options-does-not-allow-override-3.patch8.45 KB

First patch without tests.

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
CreditAttribution: nikathone commented
FileSize
3156731-4.patch15.26 KB
interdiff_3_4.txt5.75 KB

Here is an initial patch with tests. Unfortunately I started the test as unit test then I discovered that the chart element due to the plugin system integration it will need to be at least a kernel test. I will continue on this tomorrow evening and switch to kernel tests.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

I've been playing a bit more, trying to customize a few charts, and I've found the NestedArray::mergeDeepArray() mechanisme to merge raw_options does not work well for numeric array keys, because these are appended, not merged.

So it seems there's more control over the chart options by overriding js settings. :-/

GuillaumeDuveau’s picture

GuillaumeDuveau
Location Toulouse
CreditAttribution: GuillaumeDuveau at DoorInsider commented
Status: Needs review » Needs work

Same as Markus, only that I see that specifically in charts_chartjs submodule.

For instance, populateOptions() in web/modules/contrib/charts/modules/charts_chartjs/src/Plugin/chart/Library/Chartjs.php adds an X axis with:

$chart_definition['options']['scales']['xAxes'][] = [
  'stacked' => $stacking,
];

In your code if you have for instance:

'#raw_options' => [
  'options' => [
    'scales' => [
      'xAxes' => [
        [
          'gridLines' => [
            'drawOnChartArea' => FALSE,
          ],
        ],
      ],
    ],
  ],
],

Then basically mergeDeepArray adds another X axis, but also the one added by this module, and the options are not honored (the initial options take precedence)

GuillaumeDuveau’s picture

GuillaumeDuveau
Location Toulouse
CreditAttribution: GuillaumeDuveau at DoorInsider commented
Status: Needs work » Needs review
FileSize
raw-options-3156731-7.patch1.37 KB

Here's a simple patch to solve this in a super simple way: if you provide #raw_options, then by default don't add xAxes and yAxes options at all. That way, the default behaviour is the same, but you can customize things if you are a coder.

The code above in #6 now works with this patch.

What do you think? It would have to be done for other submodules of course, if this can be an acceptable solution.

andileco’s picture

andileco CreditAttribution: andileco at JSI Research & Training Institute, Inc. (JSI) for JSI Research & Training Institute, Inc. (JSI) commented
Version: 8.x-4.x-dev » 5.0.x-dev

nikathone opened merge request !38

  • andileco committed fc7652d on 5.0.x authored by nikathone 
    Issue #3156731 by nikathone, andileco, GuillaumeDuveau: #raw_options...
andileco’s picture

andileco CreditAttribution: andileco at JSI Research & Training Institute, Inc. (JSI) for JSI Research & Training Institute, Inc. (JSI) commented
Status: Needs review » Fixed

Thank you, @nikathone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.