Fix default colors for stacked bars

This addresses #101 .

Summary

Within stacked bar charts, each stack was being reversed, which resulted in the colors being reversed in that stack if the default colors were used. I removed the List.reverse and updated the downstream code to work correctly with the adjusted indexes.

Research Details

I determined that the List.reverse was being used in order to make each bar have the "correct" sectionIndex in the toBarItem function. Thus, if the toBarItem function could be updated to use the original indexes (instead of the reversed indexes), then the List.reverse could be removed and the colors would be automatically fixed.

Q: What does toBarItem use sectionIndex for? A: toBarItem uses sectionIndex in three ways:

It is used to determine if this item in the stack is the top/bottom item, and thus should have top/bottom rounding applied.
It is passed into section.extra to determine the final visual attributes to apply to the item.
It is recorded in the config.tooltipInfo in the I.Rendered value that this function returns.

Usage 1 is safe to update as long as we simply swap which condition we consider to be the "top" and which the "bottom". I have done this and changed the helper values to be named isTop/isBottom instead of isFirst/isLast.

For usages 2 and 3, it does not actually matter what the index is for each item, as long as these two usages match. To support this claim:

First sub-claim: Usage 2 will not cause any problems as long as usage 3 is in sync.

The extra field is part of the Config data type, from the Internal.Property module. It is a function that takes quite a few things and returns a list of visual modifications to make.
This field is initialized to a function that always returns an empty list by Internal.Property.property.
This field is only modified by the Internal.Property.variation function.
The Internal.Property module is not exposed to library users; thus, as long as Internal.Property.variation is not exposed to users by public modules or used in a way that cares about the index, then it is safe.
The Internal.Property.variation function is only used by Chart.variation and Chart.amongst.
- Chart.variation adds a function that ignores the sectionIndex
- Chart.amongst adds a function that uses the sectionIndex, but only to compare to the value from Item.getStackIndex. Item.getStackIndex gets the config.tooltipInfo.stack value, which is the exact value that usage 3 is writing.
Thus, usage 2 will not cause any problems if we change the sectionIndex as long as usage 3 is also updated.

Second sub-claim: Usage 3 will not cause any problems as long as usage 2 is in sync.

The config.tooltipInfo.stack value is not exposed to library users, and thus this is safe as long as internal library usage is safe.
The config.tooltipInfo.stack value is only used by the Internal.Item.getStackIndex function.
The Internal.Item.getStackIndex function is only used by Internal.Item.isSame and Chart.amongst functions
- Internal.Item.isSame is checking for equality between two values. This is safe for the update that we want to make.
- Chart.amongst is the same function that we examined above, and is safe as long as Usage 2 is updated to be in sync because it is doing equality comparison.

In conclusion, the index that was being modified by the List.reverse call (stackIndex) was being used in three ways. One required us to make a small local change, which was made. The other two do not require any changes. Once the List.reverse is removed, the default colors start working automatically.

terezka / elm-charts

Fix default colors for stacked bars #118

Summary

Research Details