D3 Updating stacked bar chart

Question

In my data there can be different number of bars in each chart. Here is a picture:

When pressing the button labeled change bars layout the view of the charts toggles between horizontal and vertical bars. For example, like this:

Since I might get bar chart stacked vertically or horizontally and there may be a different number of stacks I wasn't able use the D3 stack function and had to do most of the calculation myself.

2 more important points:

1. my code is inside VueJS framework.

2. the variable stackedXorY is 1 or 0, when it's 1 the stack is horizontal when it's 0 the stack is vertical.

A couple of questions:

1. Is there a way to use the d3 stack or some other method to create this kind of chart?

2. I noticed that when I update the chart (in handleChartLayout function) each selected stage holds the y property, even though the data itself (tasks) and stages in it don't hold it. How is that possible? It seems to me I have an issue but not sure.

Here is the an example of the data:

{  
   "id":"ACra92XfQ696H7BUCCYT",
   "isTaskFinished":true,
   "taskName":"t1",
   "initTaskTime":1536427448023,
   "endTaskTime":1536427471408,
   "taskStages":[  
      {  
         "initTime":1536427447023,
         "timeStamps":[  
            1536427449994,
            1536427453242,
            1536427456115
         ],
         "totalTime":12503
      },
      {  
         "initTime":1536427460526,
         "timeStamps":[  
            1536427465433,
            1536427470250
         ],
         "totalTime":10882
      }
   ],
   "taskTotalTime":23385
}

Here is the code:

    <template>
  <div>
    <svg
      ref="stacked-svg-tasks"
      id="stacked-svg-tasks"
      :width="svgWidth"
      :height="svgHeight"
      ></svg>
  </div>
</template>
<script>
import * as d3 from 'd3';

export default {
  name: 'TasksStackedBarChart',
  props: ['tasks', 'svgWidth', 'svgHeight', 'stackedXorY'],
  data() {
    return {
      X_AXIS_HEIGHT: 20,
      Y_AXIS_WIDTH: 50,
      svgMargin: { top: 20 },
      barChartGroup: {
        width: 0,
        height: 0,
        x: 0,
        y: 0,
      },
      GbT: 50, // gap between tasks
      stageWidth: 0,
      toalStages: 0,
      xPosOfTasks: [],
      yScale: null,
      yScaleDomainEnd: 0,
    };
  },

  created() {
    this.barChartGroup.width = this.svgWidth - this.Y_AXIS_WIDTH;
    this.barChartGroup.height = this.svgHeight - this.X_AXIS_HEIGHT;
    this.barChartGroup.x = this.Y_AXIS_WIDTH;
    this.barChartGroup.y = this.X_AXIS_HEIGHT;

    this.toalStages = this.tasks.map(v => v.taskStages.length).reduce((s, v) => s + v, 0);
    this.stageWidth = this.getStageWidth();
    this.xPosOfTasks = this.getXPosOfTask(this.GbT, this.tasks, this.stageWidth, this.stackedXorY);
    this.yScaleDomainEnd = this.getLongestTask();
    this.yScale = this.getScaleForTask(this.barChartGroup.height, this.yScaleDomainEnd);
  },

  mounted() {
    this.chartBuilder();
  },

  methods: {
    getScaleForTask(rangeEnd, domainEnd) {
      const calcRangeEnd = rangeEnd - this.svgMargin.top - this.X_AXIS_HEIGHT;
      return d3.scaleLinear()
        .range([0, calcRangeEnd])
        .domain([0, domainEnd]);
    },

    getLongestTask() {
      return d3.max(this.tasks.map(t => t.taskTotalTime));
    },

    getStageWidth() { // will keep using this. to see if it's better to work like this or i should pass parameters
      const numOfChartBars = this.stackedXorY ? this.toalStages : this.tasks.length;
      return Math.floor((this.svgWidth - (this.GbT * (this.tasks.length + 1))) / numOfChartBars);
    },

    getPosOfStage(stage, stageIndex) {
      const x = stageIndex * this.stageWidth * this.stackedXorY;
      // this sets the bars at the bottom than this.yScale(s.y) moves them accordingly
      const y = this.barChartGroup.height - this.yScale(stage.totalTime) - ((1 - this.stackedXorY) * this.yScale(stage.y));
      return `translate(${x}, ${y})`;
    },

    getXPosOfTask(gapBetweenTasks, tasks, widthOfBar, stackedYorX) {
      const newXPosOfTasks = [0]; // [gapBetweenTasks + 50];

      for (let c = 0; c < tasks.length - 1; c += 1) {
        const barsInTask = stackedYorX ? tasks[c].taskStages.length : 1;
        const taskXPos = gapBetweenTasks + (barsInTask * widthOfBar) + newXPosOfTasks[c];
        newXPosOfTasks.push(taskXPos);
      }
      return newXPosOfTasks;
    },

    cumulativeTimeOfStages(taskStages) {
      return taskStages.reduce((sums, curItem) => {
        const newSum = sums[sums.length - 1] + curItem.totalTime;
        sums.push(newSum);
        return sums;
      }, [0]);
    },

    reconstructedStageData(taskStages) {
      const cumulativeSums = this.cumulativeTimeOfStages(taskStages);
      return taskStages.map((stage, i) => ({ y: cumulativeSums[i], ...stage }));
    },

    chartBuilder() {
      // clearing svg
      d3.selectAll('#stacked-svg-tasks').selectAll('g').remove();

      const svg = d3.select('#stacked-svg-tasks');
      // adding Y axis
      const yAxisScale = d3
        .scaleTime()
        .domain([this.yScaleDomainEnd, 0])
        .range([0, this.svgHeight - this.X_AXIS_HEIGHT - this.svgMargin.top]);
      const yAxis = d3.axisLeft().scale(yAxisScale);

      const barChartGroup = svg
        .selectAll('g')
        .data([this.tasks])
        .enter()
        .append('g')
        .attr('class', 'bar-chart-group')
        .attr('transform', `translate(${this.barChartGroup.x},${this.barChartGroup.y})`);

      const taskGroups = barChartGroup
        .selectAll('g')
        .data(t => t)
        .enter()
        .append('g')
        .attr('class', (t, i) => `bar${i}`)
        .attr('transform', (t, i) => `translate(${this.xPosOfTasks[i]},0)`);

      const stageGroups = taskGroups
        .selectAll('g')
        .data(t => this.reconstructedStageData(t.taskStages))
        .enter()
        .append('g')
        .attr('transform', (s, i) => this.getPosOfStage(s, i))
        .append('rect')
        .attr('width', this.stageWidth)
        .attr('height', s => this.yScale(s.totalTime))
        .attr('fill', (d, i) => (i % 2 === 0 ? '#66ccff' : '#99ff66'))
        .attr('style', 'stroke:rgb(150,150,150);stroke-width:2');
    },

    handleChartLayout() {
      const svg = d3.select('#stacked-svg-tasks');

      this.tasks.forEach((task, taskIndex) => {
        svg
          .selectAll(`.bar${taskIndex}`)
          .data([task])
          .transition()
          .duration(750)
          .attr('transform', `translate(${this.xPosOfTasks[taskIndex]}, 0)`)
          .selectAll('g')
          .attr('transform', (s, i) => this.getPosOfStage(s, i))
          .selectAll('rect')
          .attr('width', this.stageWidth);
      });
    },
  },

  watch: {
    stackedXorY() {
      this.stageWidth = this.getStageWidth();
      this.xPosOfTasks = this.getXPosOfTask(this.GbT, this.tasks, this.stageWidth, this.stackedXorY);
      this.handleChartLayout();
    },
  },
};
</script>

Answer 1

Overall Feedback

In general, the code looks good and is fairly easy to read. Most of the methods are concise and would lend themselves well to unit tests. I like the use of const by default for variables and only using let for the iterator variable c.

I do have a few recommendations described below that should improve certain aspects of it.

Recommendations

Make id attribute unique

The id attribute “must be unique in the whole document”¹, so if you reuse that component, make the value for that attribute unique. It could be a parameter, use a static value (e.g. "id_" + count++), etc.

Avoid the Toll cost of crossing the DOM bridge

”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
    - John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post

In your code I see two occurrences of d3.select('#stacked-svg-tasks') and one of d3.selectAll('#stacked-svg-tasks'). The last expression obviously works but the difference between .select() and .selectAll() is that the former uses document.querySelector() whereas the latter uses document.querySelectorAll(), which returns a NodeList instead of a single element - somewhat overkill for getting an element using the id attribute.

In the d3 documentation for .select(selector) we see the following:

If the selector is not a string, instead selects the specified node; this is useful if you already have a reference to a node²

Because you are using VueJS and have ref="stacked-svg-tasks" on the <svg> tag in the template, you have a reference to the target node! You can use $refs instead of querying the DOM each time to select that element.

So lines like this

const svg = d3.select("#stacked-svg-tasks");

Could be updated like this:

const svg = d3.select(this.$refs['stacked-svg-tasks']);

I would recommend switching to camelCase or at least something without the hyphens -

<svg 
  ref="stackedSvgTasks"

Then the dot notation (instead of bracket notation) can be used like below:

const svg = d3.select(this.$refs.stackedSvgTasks);

Additionally, that reference svg could be stored when the component is mounted (e.g. in the mounted method) and re-used in other methods.

---

Don't Repeat d3 chains over and over again

I know that idiomatic d3.js code typically has chained calls to methods like .selectAll(), .data(), .enter(), etc. And in the chartBuilder() method I see quite a few of those redundant chains. There are at least a few places where those common chains could be abstracted to a method. For instance, if a method was added like:

applyDataToGroup(element, dataMap) {
  return element
    .selectAll('g')
    .data(dataMap)
    .enter()
    .append("g");
}

Then that could be used to greatly simplify the assignment of barChartGroup, taskGroupsand stageGroups:

const barChartGroup = this.applyDataToGroup(this.svg, [this.tasks])
  .attr("class", "bar-chart-group")
  .attr(
    "transform",
    `translate(${this.barChartGroup.x},${this.barChartGroup.y})`
  );

const taskGroups = this.applyDataToGroup(barChartGroup, t => t)
  .attr("class", (t, i) => `bar${i}`)
  .attr("transform", (t, i) => `translate(${this.xPosOfTasks[i]},0)`);

const stageGroupDataMapping = t => this.reconstructedStageData(t.taskStages);
const stageGroups = this.applyDataToGroup(taskGroups, stageGroupDataMapping)
  .attr("transform", (s, i) => this.getPosOfStage(s, i))
  .append("rect")
  .attr("width", this.stageWidth)
  .attr("height", s => this.yScale(s.totalTime))
  .attr("fill", (d, i) => (i % 2 === 0 ? "#66ccff" : "#99ff66"))
  .attr("style", "stroke:rgb(150,150,150);stroke-width:2");

While it would require adding two lines (to declare the method) and a couple extra function calls, it could reduce those 27 lines to the 19 above. To make that method simpler each of those blocks could be abstracted into separate methods.

¹_{https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id}
²_{https://github.com/d3/d3-selection#select}