Layout service supports configuring box order

[etimberg]

Resolves #3388

The layout system now supports an order property which is used to align left and right boxes. A higher value is further away from the chart. This should not change the axis positions, and I did not observe any differences. It may be prudent to wait until v2.6 to include this.

After Fix

[etimberg]

I did this documentation method based on http://stackoverflow.com/a/6460748 because it generated less docs than doing something to define an interface. Once we have gulp docs in, I can test this and see what it generates

[simonbrunel]

Indeed, that works well for nested arguments, but I think here it's really an interface and it's quite weird to have these left, top, right, bottom properties part of the function inputs. This method expects an object that implements the IBox (or ILayoutBox or ILayoutItem) interface, as we have for IPlugin, IPlatform or IEvent. Also more convenient if we need to refer to that interface from somewhere in the doc.

Actually, it doesn't require so much more doc if you define the interface as I did for IEvent.

[etimberg]

Yeah, I will update to follow the same way as IEvent

[etimberg]

Not sure about this. sort is not stable, so I don't think this guarantees the order of boxes. It might be better so simply set box.order = chartInstance.boxes.length so that it's more likely to be a unique value.

[simonbrunel]

Default order should be 0, because boxes can be registered from anywhere, so it would be impossible to really control the box order:

addBox(chart, {});              // foo (index 0)
addBox(chart, {order: 1});      // bar (index 1)
addBox(chart, {});              // xyz (index 2)

Effective order is:

• case default to 0: [foo, xyz, bar] (expected)
• case auto-increment: [foo, bar, xyz]

Instead, we could use the box index as second sort criteria to obtain a stable sort:

function sort(array, reverse) {
    return array.map(function(v, i) {
        v._tmp_index_ = i;
    })
    .sort(function (a, b) {
        var v0 = reverse? b : a;
        var v1 = reverse? a : b;
        return v0.order === v1.order?
            v0._tmp_index_ - v1._tmp_index_ :
            v0.order - v1.order;
    })
    .map(function(v) {
        delete v._tmp_index_;
    });
}

update: function() {
    sort(leftBoxes, false);
    sort(rightBoxes, true);
    sort(topBoxes, false);
    sort(bottomBoxes, true);
}

Not super efficient, but I don't think there will be so much boxes to sort!

[etimberg]

Should we apply this to top and bottom boxes as well? If so, we need to change the sorts just above here

[simonbrunel]

I think so, maybe options.fullWidth should not be used anymore to control the box order.

[etimberg]

That's probably a good idea. I will make that change

[simonbrunel]

Default order should be 0, because boxes can be registered from anywhere, so it would be impossible to really control the box order:

addBox(chart, {});              // foo (index 0)
addBox(chart, {order: 1});      // bar (index 1)
addBox(chart, {});              // xyz (index 2)

Effective order is:

• case default to 0: [foo, xyz, bar] (expected)
• case auto-increment: [foo, bar, xyz]

Instead, we could use the box index as second sort criteria to obtain a stable sort:

function sort(array, reverse) {
    return array.map(function(v, i) {
        v._tmp_index_ = i;
    })
    .sort(function (a, b) {
        var v0 = reverse? b : a;
        var v1 = reverse? a : b;
        return v0.order === v1.order?
            v0._tmp_index_ - v1._tmp_index_ :
            v0.order - v1.order;
    })
    .map(function(v) {
        delete v._tmp_index_;
    });
}

update: function() {
    sort(leftBoxes, false);
    sort(rightBoxes, true);
    sort(topBoxes, false);
    sort(bottomBoxes, true);
}

Not super efficient, but I don't think there will be so much boxes to sort!

[simonbrunel]

Indeed, that works well for nested arguments, but I think here it's really an interface and it's quite weird to have these left, top, right, bottom properties part of the function inputs. This method expects an object that implements the IBox (or ILayoutBox or ILayoutItem) interface, as we have for IPlugin, IPlatform or IEvent. Also more convenient if we need to refer to that interface from somewhere in the doc.

Actually, it doesn't require so much more doc if you define the interface as I did for IEvent.

[simonbrunel]

We shouldn't need to define @method since it's part of the Chart.layoutService object.

[simonbrunel]

Why are there some options under box.options and some others directly under box?

[simonbrunel]

Should be {Chart.Controller} to bind that parameter to the controller doc.

[simonbrunel]

I think so, maybe options.fullWidth should not be used anymore to control the box order.

[simonbrunel]

Can we rename order into weight instead?

[etimberg]

I will change order to weight, apply it to the top and bottom boxes as well. I will also update the documentation and update the title to use the new weight field

[etimberg]

@simonbrunel when you get a chance, can you review this after the changes?

[simonbrunel]

Should we add unit tests?

[etimberg]

@simonbrunel I'll add some tests to ensure that the sorting and stuff works as expected.