FEATURE: Add chart option to polls (PR #8352)

GitHub

You’ve signed the CLA, markvanlan. Thank you! This pull request is ready for review.

Can we get screenshots of that awesome feature?

Can we get screenshots of that awesome feature?

This is going to get much more complex over the coming days.

Here is a very early preview:

Screenshot from 2019-11-14 13-16-20@2x

2 Likes

this is going to be deprecated.

The preferred solution is to set chartType with an initial value

            .filter(Boolean)

we don’t user var anymore in the codebase

        type === "number" || attrs.poll.chart !== "pie"

guess you could replace directly on fieldName here, that intermediate assignment doesn’t seem necessary

is this clearfix necessary? can you try to use flexbox and not rely on this kind of css tricks?

is jquery needed for this?

I think last arg here should be an option to make it explicit=

const chartConfig = pieChartConfig(data, labels, { aspectRatio: 1.2 });

wouldn’t that be enough?

    if (select.length) {
          data,
      labels
      aspectRatio,
      const data = attrs.poll.options.map(o => o.votes);
      const labels = attrs.poll.options.map(o => o.html);

That’s a nitpick, feel free to ignore, but I like to write it that way:

  el && el.parentNode.removeChild(el);

would really prefer we try to code with flexbox and not floats