pissang commented on a change in pull request #12961:
URL:
https://github.com/apache/incubator-echarts/pull/12961#discussion_r471086187
##########
File path: src/model/mixin/itemStyle.ts
##########
@@ -54,15 +64,24 @@ class ItemStyleMixin {
includes?: readonly (keyof ItemStyleOption)[]
): ItemStyleProps {
const style = getItemStyle(this, excludes, includes);
- const lineDash = this.getBorderLineDash();
- lineDash && (style.lineDash = lineDash);
+ style.lineDash = this.getBorderLineDash(style.lineWidth);
return style;
}
- getBorderLineDash(this: Model): number[] {
+ getBorderLineDash(this: Model, lineWidth?: number): number[] {
const lineType = this.get('borderType');
- return (lineType === 'solid' || lineType == null) ? null
- : (lineType === 'dashed' ? [5, 5] : [1, 1]);
+ if (lineType == null || lineType === 'solid') {
+ return [];
+ }
+
+ lineWidth = lineWidth || 0;
+
+ let dashArray = this.get('borderDashArray');
+ // compatible with single number
+ if (dashArray != null && !isNaN(dashArray)) {
+ dashArray = [+dashArray];
+ }
+ return dashArray || (lineType === 'dashed' ? [5, 5] : [1, 1]);
}
Review comment:
`style` in `getVisual` is encoded in
https://github.com/apache/incubator-echarts/blob/next/src/visual/style.ts#L48.
Perhaps we can also add `['lineDash', 'borderType']` in the
`ITEM_STYLE_KEY_MAP` and `LINE_STYLE_KEY_MAP`.
Also, your comment reminds me of why the `borderType` in the next branch not
work. We need some post-processing on the style returned by the `styleMapper`
in https://github.com/apache/incubator-echarts/blob/next/src/visual/style.ts#L77
For example
```ts
// In the normalizeLineDash, it will convert lineDash: 'dashed', 'dotted' to
[5, 5], [1, 1].
normalizeLineDash(globalStyle);
```
> We added a semantic value bolder in #13013, but I found this special value
will affect the calculation of default dash/dot size and return an illegal
value NaN. Perhaps, we should change the logic about either LineView or
model/mixin/lineStyle?
Combined with this issue, I think I will prefer to do the normalization of
`lineDash` in zrender. Both in SVG and Canvas renderer.
https://github.com/ecomfe/zrender/blob/next/src/svg/graphic.ts#L110
https://github.com/ecomfe/zrender/blob/next/src/canvas/graphic.ts#L172
It will make the `lineDash` processing much easier to be consistent in the
echarts code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
