pissang commented on a change in pull request #15182:
URL: https://github.com/apache/echarts/pull/15182#discussion_r658448716
##########
File path: src/chart/bar/BarSeries.ts
##########
@@ -35,10 +35,12 @@ import { inheritDefaultOption } from '../../util/component';
import List from '../../data/List';
import { BrushCommonSelectorsForSeries } from '../../component/brush/selector';
+export type PolarBarLabelPosition = SeriesLabelOption['position']
+ | 'start' | 'insideStart' | 'middle' | 'end' | 'insideEnd';
export interface BarStateOption {
itemStyle?: BarItemStyleOption
- label?: SeriesLabelOption
+ label?: SeriesLabelOption & {position: PolarBarLabelPosition}
Review comment:
`&` operator may be wrong here.
```ts
(SeriesLabelOption['position'] | 'start' | 'insideStart' | 'middle' | 'end'
| 'insideEnd')
& SeriesLabelOption['position']
```
is still `(SeriesLabelOption['position']`
Check
https://www.typescriptlang.org/play?#code/C4TwDgpgBAglC8UDeAoK6oDMD22D8AXFAOQCWAdgM6kAmExUAPidgK7DV3EoC+KokKACEEsKADJkaDDmxEYAbWKziAXSYlKwAIYAnYA2bEI5Gtz4Dos0UKUrVQA
In `PieSeries` there is a similar scenario
https://github.com/apache/echarts/blob/51f4c65be21bfb589370a422a0fcc39d598efef3/src/chart/pie/PieSeries.ts#L61
##########
File path: src/chart/bar/BarView.ts
##########
@@ -911,34 +935,40 @@ function updateStyle(
const cursorStyle = itemModel.getShallow('cursor');
cursorStyle && (el as Path).attr('cursor', cursorStyle);
- if (!isPolar) {
- const labelPositionOutside = isHorizontal
- ? ((layout as RectLayout).height > 0 ? 'bottom' as const : 'top'
as const)
- : ((layout as RectLayout).width > 0 ? 'left' as const : 'right' as
const);
- const labelStatesModels = getLabelStatesModels(itemModel);
-
- setLabelStyle(
- el, labelStatesModels,
- {
- labelFetcher: seriesModel,
- labelDataIndex: dataIndex,
- defaultText: getDefaultLabel(seriesModel.getData(), dataIndex),
- inheritColor: style.fill as ColorString,
- defaultOpacity: style.opacity,
- defaultOutsidePosition: labelPositionOutside
- }
- );
-
- const label = el.getTextContent();
-
- setLabelValueAnimation(
- label,
- labelStatesModels,
- seriesModel.getRawValue(dataIndex) as ParsedValue,
- (value: number) => getDefaultInterpolatedLabel(data, value)
+ const labelPositionOutside = isHorizontalOrRadial
+ ? ((layout as RectLayout).height > 0 ? 'bottom' as const : 'top' as
const)
Review comment:
Seems polar is ignore here?
##########
File path: src/label/sectorLabel.ts
##########
@@ -0,0 +1,234 @@
+import {RectLike, Sector} from 'zrender';
+import {calculateTextPosition, TextPositionCalculationResult} from
'zrender/src/contain/text';
+import {BuiltinTextPosition, TextAlign, TextVerticalAlign} from
'zrender/src/core/types';
+import {isArray} from 'zrender/src/core/util';
+import {ElementCalculateTextPosition, ElementTextConfig} from
'zrender/src/Element';
+
+export type SectorTextPosition = BuiltinTextPosition
+ | 'startAngle' | 'insideStartAngle'
+ | 'endAngle' | 'insideEndAngle'
+ | 'middle'
+ | 'startArc' | 'insideStartArc'
+ | 'endArc' | 'insideEndArc'
+ | (number | string)[];
+
+export type SectorLike = {
+ cx: number
+ cy: number
+ r0: number
+ r: number
+ startAngle: number
+ endAngle: number
+ clockwise: boolean
+};
+
+export function createSectorCalculateTextPosition<T extends (string | (number
| string)[])>(
+ positionMapping: (seriesLabelPosition: T) => SectorTextPosition
+): ElementCalculateTextPosition {
+ return function (
+ this: Sector,
+ out: TextPositionCalculationResult,
+ opts: {
+ position?: SectorTextPosition
+ distance?: number
+ global?: boolean
+ },
+ boundingRect: RectLike
+ ) {
+ const textPosition = opts.position;
+
+ if (!textPosition || textPosition instanceof Array) {
+ return calculateTextPosition(
+ out,
+ opts as ElementTextConfig,
+ boundingRect
+ );
+ }
+
+ const mappedSectorPosition = positionMapping(textPosition as T);
+ const distance = opts.distance != null ? opts.distance : 5;
+ const sector = this.shape;
+ const cx = sector.cx;
+ const cy = sector.cy;
+ const r = sector.r;
+ const r0 = sector.r0;
+ const middleR = (r + r0) / 2;
+ const startAngle = sector.clockwise ? sector.startAngle :
sector.endAngle;
Review comment:
I think we should always choose `sector.startAngle` here and put the
clockwise logic in the mapping methods of `BarView`. Reasons:
1. The name of position is `startAngle` and `insideStartAngle`
2. Other charts may not have this `const clockwise = layout.startAngle <
layout.endAngle` logic like `BarView` do. It's totally legal if they only use
`clockwise` to control the direction. In this case, `startAngle` and `endAngle`
should not be reversed.
--
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]
