plainheart commented on a change in pull request #14214: URL: https://github.com/apache/echarts/pull/14214#discussion_r570666229
########## File path: src/component/tooltip/TooltipRichContent.ts ########## @@ -106,7 +106,10 @@ class TooltipRichContent { fill: tooltipModel.get(['textStyle', 'color']), padding: getPaddingFromTooltipModel(tooltipModel, 'richText'), verticalAlign: 'top', - align: 'left' + align: 'left', + width: +tooltipModel.get(['textStyle', 'width']) || null, Review comment: `height` should also be specified. ########## File path: src/component/tooltip/TooltipRichContent.ts ########## @@ -106,7 +106,10 @@ class TooltipRichContent { fill: tooltipModel.get(['textStyle', 'color']), padding: getPaddingFromTooltipModel(tooltipModel, 'richText'), verticalAlign: 'top', - align: 'left' + align: 'left', + width: +tooltipModel.get(['textStyle', 'width']) || null, + overflow: tooltipModel.get(['textStyle', 'overflow']), + ellipsis: tooltipModel.get(['textStyle', 'ellipsis']) Review comment: `lineOverflow` needs to be added. ########## File path: src/util/types.ts ########## @@ -1345,6 +1345,8 @@ export interface CommonTooltipOption<FormatterParams> { // Available when renderMode is html decoration?: string + overflow?: 'none' | 'truncate' | 'break' | 'breakAll' + ellipsis?: string Review comment: The type definitions for `overflow` and `ellipsis` here are inappropriate and seems redundant. We can pick the `overflow` property from `LabelOption`. As for the `ellipsis` and `lineOverflow`, they can also be picked from the `TextStyleProps` in zrender. I think they are just missing in `LabelOption`. We should add them. ```ts export interface LabelOption extends TextCommonOption { overflow?: TextStyleProps['overflow'] ellipsis?: TextStyleProps['ellipsis'] lineOverflow?: TextStyleProps['lineOverflow'] } ``` Then add support for `ellipsis` after https://github.com/apache/echarts/blob/master/src/label/labelStyle.ts#L416-L419 ```ts const ellipsis = textStyleModel.get('ellipsis'); if (ellipsis) { textStyle.ellipsis = ellipsis; } const lineOverflow = textStyleModel.get('lineOverflow'); if (lineOverflow) { textStyle.lineOverflow = lineOverflow; } ``` But there is a TODO https://github.com/ecomfe/zrender/blob/master/src/graphic/helper/parseText.ts#L206-L213 So `lineOverflow: truncate` won't be truncated by `ellipsis` characters. We can fix it later. ########## File path: src/component/tooltip/TooltipHTMLContent.ts ########## @@ -358,6 +358,34 @@ class TooltipHTMLContent { + `border-color: ${nearPointColor};` + (tooltipModel.get('extraCssText') || ''); + const userWidth = tooltipModel.get(['textStyle', 'width']); + if (userWidth != null) { + el.style.cssText += `;width:${userWidth}px;`; + // `text-overflow_string` has very humble compatibility + // shttps://caniuse.com/mdn-css_properties_text-overflow_string + const ellipsis = tooltipModel.get(['textStyle', 'ellipsis']); + const userOverflow = tooltipModel.get(['textStyle', 'overflow']); + switch (userOverflow) { + case 'truncate': + el.style.cssText += ';overflow:hidden;' + + `text-overflow:${ellipsis != null ? `\'${ellipsis}\'` : 'ellipsis'};` + + 'white-space:nowrap;'; + break; + + case 'break': + el.style.cssText += ';word-break:break-word;white-space:normal;'; + break; + + case 'breakAll': + el.style.cssText += ';word-break:break-all;white-space:normal;'; + break; + + default: + el.style.cssText += ''; + break; + } + } Review comment: I'm thinking this logic can be simplified a bit. Just FYI. ```js const ellipsis = retrieve(tooltipModel.get(['textStyle', 'ellipsis']), 'ellipsis'); const userOverflow = tooltipModel.get(['textStyle', 'overflow']); if (userOverflow) { let overflowStyle; if (userOverflow === 'truncate') { overflowStyle = `overflow:hidden;text-overflow:${ellipsis};white-space:nowrap;` } else { const breakMode = userOverflow === 'break' ? 'break-word' : userOverflow === 'breakAll' ? 'break-all' : ''; breakMode && (overflowStyle = `word-break:${breakMode};white-space:normal;`); } el.style.cssText += overflowStyle; } ``` ########## File path: src/component/tooltip/TooltipHTMLContent.ts ########## @@ -358,6 +358,34 @@ class TooltipHTMLContent { + `border-color: ${nearPointColor};` + (tooltipModel.get('extraCssText') || ''); + const userWidth = tooltipModel.get(['textStyle', 'width']); + if (userWidth != null) { + el.style.cssText += `;width:${userWidth}px;`; + // `text-overflow_string` has very humble compatibility + // shttps://caniuse.com/mdn-css_properties_text-overflow_string + const ellipsis = tooltipModel.get(['textStyle', 'ellipsis']); + const userOverflow = tooltipModel.get(['textStyle', 'overflow']); + switch (userOverflow) { + case 'truncate': + el.style.cssText += ';overflow:hidden;' + + `text-overflow:${ellipsis != null ? `\'${ellipsis}\'` : 'ellipsis'};` + + 'white-space:nowrap;'; + break; + + case 'break': + el.style.cssText += ';word-break:break-word;white-space:normal;'; + break; + + case 'breakAll': + el.style.cssText += ';word-break:break-all;white-space:normal;'; + break; + + default: + el.style.cssText += ''; + break; + } + } Review comment: I'm thinking this logic can be simplified a bit. Just FYI. ```js const ellipsis = retrieve(tooltipModel.get(['textStyle', 'ellipsis']), 'ellipsis'); const userOverflow = tooltipModel.get(['textStyle', 'overflow']); if (userOverflow) { let overflowStyle = ''; if (userOverflow === 'truncate') { overflowStyle = `overflow:hidden;text-overflow:${ellipsis};white-space:nowrap;` } else { const breakMode = userOverflow === 'break' ? 'break-word' : userOverflow === 'breakAll' ? 'break-all' : ''; breakMode && (overflowStyle = `word-break:${breakMode};white-space:normal;`); } el.style.cssText += overflowStyle; } ``` ########## File path: src/util/types.ts ########## @@ -1345,6 +1345,8 @@ export interface CommonTooltipOption<FormatterParams> { // Available when renderMode is html decoration?: string + overflow?: 'none' | 'truncate' | 'break' | 'breakAll' + ellipsis?: string Review comment: The type definitions for `overflow` and `ellipsis` here are inappropriate and seems redundant. We can pick the `overflow` property from `LabelOption`. As for the `ellipsis` and `lineOverflow`, they can also be picked from the `TextStyleProps` in zrender. I think they are just missing in `LabelOption`. We should add them. ```ts export interface LabelOption extends TextCommonOption { overflow?: TextStyleProps['overflow'] ellipsis?: TextStyleProps['ellipsis'] lineOverflow?: TextStyleProps['lineOverflow'] } ``` Then add support for `ellipsis` after https://github.com/apache/echarts/blob/master/src/label/labelStyle.ts#L416-L419 ```ts const ellipsis = textStyleModel.get('ellipsis'); if (ellipsis) { textStyle.ellipsis = ellipsis; } const lineOverflow = textStyleModel.get('lineOverflow'); if (lineOverflow) { textStyle.lineOverflow = lineOverflow; } ``` But there is a TODO https://github.com/ecomfe/zrender/blob/master/src/graphic/helper/parseText.ts#L206-L213 So `lineOverflow: truncate` won't be truncated by `ellipsis` characters. We can fix it later. Besides, `lineOverflow` may not be working in HTML rendering mode. ########## File path: src/util/types.ts ########## @@ -1345,6 +1345,8 @@ export interface CommonTooltipOption<FormatterParams> { // Available when renderMode is html decoration?: string + overflow?: 'none' | 'truncate' | 'break' | 'breakAll' + ellipsis?: string Review comment: The type definitions for `overflow` and `ellipsis` here are inappropriate and seems redundant. We can pick the `overflow` property from `LabelOption`. As for the `ellipsis` and `lineOverflow`, they can also be picked from the `TextStyleProps` in zrender. I think they are just missing in `LabelOption`. We should add them. ```ts export interface LabelOption extends TextCommonOption { overflow?: TextStyleProps['overflow'] ellipsis?: TextStyleProps['ellipsis'] lineOverflow?: TextStyleProps['lineOverflow'] } ``` Then add support for `ellipsis` after https://github.com/apache/echarts/blob/master/src/label/labelStyle.ts#L416-L419 ```ts const ellipsis = textStyleModel.get('ellipsis'); if (ellipsis) { textStyle.ellipsis = ellipsis; } const lineOverflow = textStyleModel.get('lineOverflow'); if (lineOverflow) { textStyle.lineOverflow = lineOverflow; } ``` But there is a TODO https://github.com/ecomfe/zrender/blob/master/src/graphic/helper/parseText.ts#L206-L213 So `lineOverflow: truncate` won't be truncated by `ellipsis` characters. We can fix it later. Besides, `lineOverflow` may not be working in HTML rendering mode. Though there is a CSS property `line-clamp`, it is an unsupported WebKit property and it may have poor compatibility in some browsers. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org For additional commands, e-mail: commits-h...@echarts.apache.org