Change borderCapStyle and borderJoinStyle defaults to be compatible with SKIA canvas

[mlohbihler]

Setting these defaults prevents errors when using annotations in the skia canvas context. See Issue for more details: #938

[LeeLenaleee]

Change LGTM, but it seems the CI fails

[mlohbihler]

Change LGTM, but it seems the CI fails

I'll have a look.

[mlohbihler]

I think i need help with this. test-integration works locally for me, but i'm on mac. Not sure why it would be failing on ubuntu.

Edit: Actually, it looks like test-integration succeeded, but then the process failed with no further messages.

[mlohbihler]

I think i need help with this. test-integration works locally for me, but i'm on mac. Not sure why it would be failing on ubuntu.

Edit: Actually, it looks like test-integration succeeded, but then the process failed with no further messages.

Actually, i'm getting the same build failure on the master branch. @LeeLenaleee can you check?

[LeeLenaleee]

I will look into it this weekend

[stockiNail]

@mlohbihler @LeeLenaleee has written in the issue, we need to take care about options fallback.

As far as I remember, with this updates we are creating a breaking change because we are stopping the fallback of start and end config to arrowHeads configuration and not to the line options, as is today.

[stockiNail]

@mlohbihler LGTM. May be you can try the branch against SKIA canvas to check if it is working as expected.

[stockiNail]

@mlohbihler the failed test has been fixed in #902. Therefore after merging of that and rebase this branch, we could re-run the failed job

[mlohbihler]

@mlohbihler the failed test has been fixed in #902. Therefore after merging of that and rebase this branch, we could re-run the failed job

@stockiNail nice. Thanks for this.

[stockiNail]

@mlohbihler if you will rebase and push again, we can go on. Take your time. Thanks a lot!

[mlohbihler]

@mlohbihler if you will rebase and push again, we can go on. Take your time. Thanks a lot!

Build checks are passing now, but i guess with the (net zero) playing around i did i lost my approval. Re-requested.

[stockiNail]

Apologize if I hadn't time before to have a look.

The changes are adding options not used by those annotations therefore I would prefer to change only the setBorderStyle (reverting the current changes of PR) as following:

export function setBorderStyle(ctx, options) {
  if (options && options.borderWidth) {
    ctx.lineCap = options.borderCapStyle || 'butt';
    ctx.setLineDash(options.borderDash);
    ctx.lineDashOffset = options.borderDashOffset;
    ctx.lineJoin = options.borderJoinStyle  || 'miter';
    ctx.lineWidth = options.borderWidth;
    ctx.strokeStyle = options.borderColor;
    return true;
  }
}

Let me know what you think

[mlohbihler]

Apologize if I hadn't time before to have a look.

The changes are adding options not used by those annotations therefore I would prefer to change only the setBorderStyle (reverting the current changes of PR) as following:

export function setBorderStyle(ctx, options) {
  if (options && options.borderWidth) {
    ctx.lineCap = options.borderCapStyle || 'butt';
    ctx.setLineDash(options.borderDash);
    ctx.lineDashOffset = options.borderDashOffset;
    ctx.lineJoin = options.borderJoinStyle  || 'miter';
    ctx.lineWidth = options.borderWidth;
    ctx.strokeStyle = options.borderColor;
    return true;
  }
}

Let me know what you think

That change seems to work for me. Note that there are other instances of these properties getting set. Not sure if they can be removed too. E.g. see https://github.com/mlohbihler/chartjs-plugin-annotation/blob/0bce23d13a8db9aa9c58a80e5fc2929b4dfbab71/src/types/box.js#L40

[stockiNail]

That change seems to work for me. Note that there are other instances of these properties getting set. Not sure if they can be removed too. E.g. see https://github.com/mlohbihler/chartjs-plugin-annotation/blob/0bce23d13a8db9aa9c58a80e5fc2929b4dfbab71/src/types/box.js#L40

@mlohbihler the joinStyle and capstyle are used for setting on canvas only in the point that you have changed.
The other istances are only in the options defintions of the annotations in order to enable the options settings from the user.