100pah commented on code in PR #20977:
URL: https://github.com/apache/echarts/pull/20977#discussion_r2128424459
##########
src/label/labelStyle.ts:
##########
@@ -395,15 +395,18 @@ function setTextStyleCommon(
let richResult: TextStyleProps['rich'];
if (richItemNames) {
richResult = {};
+ const richInheritPlainLabel =
textStyleModel.get('richInheritPlainLabel') !== false;
for (const name in richItemNames) {
if (richItemNames.hasOwnProperty(name)) {
// Cascade is supported in rich.
- const richTextStyle = textStyleModel.getModel(['rich', name]);
+ const richTextStyle = textStyleModel.getModel(['rich', name],
richInheritPlainLabel && textStyleModel);
// In rich, never `disableBox`.
Review Comment:
I found that after this changing, `test/pie-label-alignTo-adjust.html`
throws JS Error in zrender.
<img width="594" alt="image"
src="https://github.com/user-attachments/assets/eaf058d0-4a7f-4b2b-a4fa-aba3ae8033df";
/>
The root cause of that Error is related to zr incorrect impl (I'm fixing
it.), but not caused by this PR.
However, this PR changes the inheritance behavior, causing the rich style to
inherit `width` from the outermost label style, which trigger the bug code in
zr.
Through this case, I realized that:
+ those props, `width`, `height`, `padding`, `margin`, `tag`,
`backgroundColor`, `borderColor`, `borderWidth`, `borderRadius`, `shadowColor`,
`shadowBlur`, `shadowOffsetX`, `shadowOffsetY` (in `TextStylePropsPart`), are
inappropriate to inherit.
+ If the default label style (`static defaultOption = ...`) has props (such
as, `align` `verticalAlign`, and in `GauseSeries`), we change the inheritance
behavior, that may cause a breaking change. Also need to consider that some
default style is set in emphasis state. And users have to reset them in each
rich style to correct the style - that might cause the config more complicated
than no inheritance.
Thus should we only conservatively only allow inheritance in
`TEXT_PROPS_WITH_GLOBAL`, i.e., `'fontStyle', 'fontWeight', 'fontSize',
'fontFamily', 'textShadowColor', 'textShadowBlur', 'textShadowOffsetX',
'textShadowOffsetY'` - they are meant to be inheritable (has been inheritable
from global option).
Moreover, I found that it's not a correct way to implements the inheritance
by
```ts
const richTextStyle = textStyleModel.getModel(
['rich', name],
richInheritPlainLabel !== false ? textStyleModel : void 0
);
```
See `test/pie-dataView.html`, that breaks the rich text in pie, because in
that case the `textStyleModel` is the model of each item and the `rich` should
be fetched from the original `textStyleModel.parent`.
Regarding impl,
```ts
// revert to
const richTextStyle = textStyleModel.getModel(['rich', name]);
// and pass `textStyleModel` into `setTokenTextStyle`
const plainTextStyle = textStyleModel.option ?
textStyleModel.option.textStyle : null;
setTokenTextStyle(
richResult[name] = {}, richTextStyle, globalTextStyle, plainTextStyle,
richInheritPlainLabel, opt, isNotNormal, isAttached, false, true
);
// and in `setTokenTextStyle`:
function setTokenTextStyle(
// ...
plainTextStyle: LabelOption | NullUndefined,
richInheritPlainLabel: boolean,
// ...
) {
// ...
for (let i = 0; i < TEXT_PROPS_WITH_GLOBAL.length; i++) {
const key = TEXT_PROPS_WITH_GLOBAL[i];
// props width, height, padding, margin, tag, backgroundColor,
borderColor,
// borderWidth, borderRadius, shadowColor, shadowBlur, shadowOffsetX,
shadowOffsetY
// may inappropriate to inherit from plainTextStyle.
// And if some props is specified in default options, users may have
to reset them one by one.
// Therefore, we only allow these props to inherit from
plainTextStyle.
// `richInheritPlainLabel` is switch for backward compatibility
const val = richInheritPlainLabel !== false
? retrieve3(textStyleModel.getShallow(key), plainTextStyle ?
plainTextStyle[key] : null, globalTextStyle[key]);
: retrieve2(textStyleModel.getShallow(key),
globalTextStyle[key]);
if (val != null) {
(textStyle as any)[key] = val;
}
}
}
```
--
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]
