Fix label for missing values in CategoricalStackedDistributionlCellRenderer

[thinkh]

closes datavisyn/tdp_core#144

prerequisites:

• branch is up-to-date with the branch to be merged with, i.e. develop
• build is successful
• code is cleaned up and formatted
• tested with Firefox 52, Firefox 57+, Chrome 64+, MS Edge 16+

Summary

Debugging datavisyn/tdp_core#144 revealed that the missing value div is rendered two times:

The first one is added right after mapping the categories:

https://github.com/datavisyn/lineupjs/blob/c0a9ba907f6cb1cd64024958453331a206314c33/src/renderer/CategoricalStackedDistributionlCellRenderer.ts#L91

The other one is introduced in the renderer template.

The duplication leads to a problem, as the renderer iterates over the DOM collection and there are less categories available.

https://github.com/datavisyn/lineupjs/blob/c0a9ba907f6cb1cd64024958453331a206314c33/src/renderer/CategoricalStackedDistributionlCellRenderer.ts#L98-L103

I removed the last div from the renderer template and now the number of children is correct and the bug is fixed:

@sgratzl Please have a look, if my fix has some side effects. Because you know best, if the removed missing value div is used elsewhere.

[sgratzl]

The duplication leads to a problem, as the renderer iterates over the DOM collection and there are less categories available.

but it checks:

const y = i >= hist.length ? missing : hist[i].y; 

thus for the last one it takes the missing value

[thinkh]

Correct. I get the documented error, since there are 44 DOM elements are available, but only 43 items are available in cat. So either the last DOM element for the missing values must be replaced or removed.

[sgratzl]

so you actually wanna have

const label = i >= cats.length ? 'Missing Values' : cats[i].label; 

[thinkh]

Yes! That works, even though there are still more DOM elements than categories.

[sgratzl]

on purpose. if you have a group with 50% missing values then stacked bar should also just go to 50% of the width.

[thinkh]

I see. Thanks for the fix and the explanation. 👍

[thinkh]

@sgratzl Should I merge the PR or will you do it?