pissang commented on a change in pull request #16034:
URL: https://github.com/apache/echarts/pull/16034#discussion_r764024437
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
+ layout: LabelLayout,
+ availableWidth: number,
+ forceRecalculate: boolean = false
+) {
+ const label = layout.label;
+ const style = label.style;
+ const textRect = layout.rect;
+ const bgColor = style.backgroundColor;
+ const padding = style.padding as number[];
+ const paddingH = padding ? padding[1] + padding[3] : 0;
+ const overflow = style.overflow;
+
+ // textRect.width already contains paddingH if bgColor is set
+ const oldOuterWidth = textRect.width + (bgColor ? 0 : paddingH);
+ if (availableWidth < oldOuterWidth
+ || forceRecalculate && layout.labelStyleWidth == null
+ ) {
+ const oldHeight = textRect.height;
+ if (overflow && overflow.match('break')) {
+ const oldEllipsis = style.ellipsis;
+
+ // Temporarily set background to be null to calculate
+ // the bounding box without backgroud.
+ label.setStyle('backgroundColor', null);
+ label.setStyle('ellipsis', '');
+
+ // Set constraining width
+ label.setStyle('width', availableWidth - paddingH);
+
+ if (layout.labelStyleWidth == null) {
+ // This is the real bounding box of the text without padding
+ const innerRect = label.getBoundingRect();
+ innerRect.applyTransform(label.getComputedTransform());
Review comment:
There is no need to applyTransform because only scale transform will
affect the width, which doesn't exits in pie labels
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
+ layout: LabelLayout,
+ availableWidth: number,
+ forceRecalculate: boolean = false
+) {
+ const label = layout.label;
+ const style = label.style;
+ const textRect = layout.rect;
+ const bgColor = style.backgroundColor;
+ const padding = style.padding as number[];
+ const paddingH = padding ? padding[1] + padding[3] : 0;
+ const overflow = style.overflow;
+
+ // textRect.width already contains paddingH if bgColor is set
+ const oldOuterWidth = textRect.width + (bgColor ? 0 : paddingH);
+ if (availableWidth < oldOuterWidth
+ || forceRecalculate && layout.labelStyleWidth == null
+ ) {
+ const oldHeight = textRect.height;
+ if (overflow && overflow.match('break')) {
+ const oldEllipsis = style.ellipsis;
+
+ // Temporarily set background to be null to calculate
+ // the bounding box without backgroud.
+ label.setStyle('backgroundColor', null);
+ label.setStyle('ellipsis', '');
+
+ // Set constraining width
+ label.setStyle('width', availableWidth - paddingH);
+
+ if (layout.labelStyleWidth == null) {
+ // This is the real bounding box of the text without padding
+ const innerRect = label.getBoundingRect();
+ innerRect.applyTransform(label.getComputedTransform());
+ label.setStyle('width', Math.ceil(innerRect.width));
+ }
+
+ label.setStyle('backgroundColor', bgColor);
+ label.setStyle('ellipsis', oldEllipsis);
+ }
+ else {
+ const availableInnerWidth = availableWidth - paddingH;
+ if (availableWidth < oldOuterWidth) {
+ // Current text is too wide, use `availableWidth` as max width.
+ label.setStyle('width', availableInnerWidth);
Review comment:
To make the code size smaller. We can use a temporary variable `newWidth`
```ts
newWidth = availableInnerWidth;
```
Then we set this `newWidth` to style at last.
```ts
label.setStyle('width', newWidth);
```
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
Review comment:
The method name should be more clear and focused on the thing it does.
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
+ layout: LabelLayout,
+ availableWidth: number,
+ forceRecalculate: boolean = false
+) {
+ const label = layout.label;
+ const style = label.style;
+ const textRect = layout.rect;
+ const bgColor = style.backgroundColor;
+ const padding = style.padding as number[];
+ const paddingH = padding ? padding[1] + padding[3] : 0;
+ const overflow = style.overflow;
+
+ // textRect.width already contains paddingH if bgColor is set
+ const oldOuterWidth = textRect.width + (bgColor ? 0 : paddingH);
+ if (availableWidth < oldOuterWidth
+ || forceRecalculate && layout.labelStyleWidth == null
+ ) {
+ const oldHeight = textRect.height;
+ if (overflow && overflow.match('break')) {
+ const oldEllipsis = style.ellipsis;
+
+ // Temporarily set background to be null to calculate
+ // the bounding box without backgroud.
+ label.setStyle('backgroundColor', null);
+ label.setStyle('ellipsis', '');
Review comment:
There is no need to set ellipsis because only `overflow: break` will
enter this branch
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
+ layout: LabelLayout,
+ availableWidth: number,
+ forceRecalculate: boolean = false
+) {
+ const label = layout.label;
+ const style = label.style;
+ const textRect = layout.rect;
+ const bgColor = style.backgroundColor;
+ const padding = style.padding as number[];
+ const paddingH = padding ? padding[1] + padding[3] : 0;
+ const overflow = style.overflow;
+
+ // textRect.width already contains paddingH if bgColor is set
+ const oldOuterWidth = textRect.width + (bgColor ? 0 : paddingH);
+ if (availableWidth < oldOuterWidth
+ || forceRecalculate && layout.labelStyleWidth == null
Review comment:
I think it's not necessary to enter this whole method if
`labelStyleWidth` is not null
##########
File path: src/chart/pie/labelLayout.ts
##########
@@ -228,6 +255,91 @@ function avoidOverlap(
}
}
+/**
+ * Set max width of each label, and then wrap each label to the max width.
+ *
+ * @param layout label layout
+ * @param availableWidth max width for the label to display
+ * @param forceRecalculate recaculate the text layout even if the current width
+ * is smaller than `availableWidth`. This is useful when the text was
previously
+ * wrapped by calling `layoutText` but now `availableWidth` changed, in which
+ * case, previous wrapping should be redo. `forceRecalculate` is ignored if
+ * `labelStyleWidth` is set.
+ */
+function layoutText(
+ layout: LabelLayout,
+ availableWidth: number,
+ forceRecalculate: boolean = false
+) {
+ const label = layout.label;
+ const style = label.style;
+ const textRect = layout.rect;
+ const bgColor = style.backgroundColor;
+ const padding = style.padding as number[];
+ const paddingH = padding ? padding[1] + padding[3] : 0;
+ const overflow = style.overflow;
+
+ // textRect.width already contains paddingH if bgColor is set
+ const oldOuterWidth = textRect.width + (bgColor ? 0 : paddingH);
+ if (availableWidth < oldOuterWidth
+ || forceRecalculate && layout.labelStyleWidth == null
+ ) {
+ const oldHeight = textRect.height;
+ if (overflow && overflow.match('break')) {
+ const oldEllipsis = style.ellipsis;
+
+ // Temporarily set background to be null to calculate
+ // the bounding box without backgroud.
+ label.setStyle('backgroundColor', null);
+ label.setStyle('ellipsis', '');
+
+ // Set constraining width
+ label.setStyle('width', availableWidth - paddingH);
+
+ if (layout.labelStyleWidth == null) {
+ // This is the real bounding box of the text without padding
+ const innerRect = label.getBoundingRect();
+ innerRect.applyTransform(label.getComputedTransform());
+ label.setStyle('width', Math.ceil(innerRect.width));
+ }
+
+ label.setStyle('backgroundColor', bgColor);
+ label.setStyle('ellipsis', oldEllipsis);
+ }
+ else {
+ const availableInnerWidth = availableWidth - paddingH;
+ if (availableWidth < oldOuterWidth) {
+ // Current text is too wide, use `availableWidth` as max width.
+ label.setStyle('width', availableInnerWidth);
Review comment:
To make the code size smaller. We can use a temporary variable `newWidth`
```ts
newWidth = availableInnerWidth;
```
Then we set this `newWidth` to style at last.
```ts
label.setStyle('width', newWidth);
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
