100pah commented on a change in pull request #12947:
URL:
https://github.com/apache/incubator-echarts/pull/12947#discussion_r457857968
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -259,16 +314,24 @@ class TooltipHTMLContent {
// this.hide();
}
- show(tooltipModel: Model<TooltipOption>) {
+ show(tooltipModel: Model<TooltipOption>, nearPointColor: ZRColor) {
clearTimeout(this._hideTimeout);
const el = this.el;
const styleCoord = this._styleCoord;
+ const offset = el.offsetHeight / 2;
+ if (zrUtil.isObject(nearPointColor) && nearPointColor.type !==
'pattern') {
+ nearPointColor = nearPointColor.colorStops[0].color;
+ }
+ else if (zrUtil.isObject(nearPointColor) && (nearPointColor.type ===
'pattern')) {
+ nearPointColor = 'transparent';
+ }
el.style.cssText = gCssText + assembleCssText(tooltipModel)
// Because of the reason described in:
//
http://stackoverflow.com/questions/21125587/css3-transition-not-working-in-chrome-anymore
// we should set initial value to `left` and `top`.
- + ';left:' + styleCoord[0] + 'px;top:' + styleCoord[1] + 'px;'
+ + ';left:' + styleCoord[0] + 'px;top:' + (styleCoord[1] - offset)
+ 'px;'
+ + `border-color: ${nearPointColor}`
Review comment:
Should be `border-color: ${nearPointColor};`, missing a `;`.
Otherwise, the following `extraCssText` will be broken.
##########
File path: src/component/axisPointer/findPointFromSeries.ts
##########
@@ -57,7 +58,20 @@ export default function (finder: {
const el = data.getItemGraphicEl(dataIndex);
const coordSys = seriesModel.coordinateSystem;
- if (seriesModel.getTooltipPosition) {
+ if (coordSys && coordSys.dataToPoint && finder.isStacked) {
+ const baseAxis = coordSys.getBaseAxis();
+ const valueAxis = coordSys.getOtherAxis(baseAxis as any);
+ const valueAxisDim = valueAxis.dim;
+ const baseAxisDim = baseAxis.dim;
+ const baseDataOffset = valueAxisDim === 'x' || valueAxisDim ===
'radius' ? 1 : 0;
+ const baseDim = data.mapDimension(baseAxisDim);
+ const stackedData = [];
+ stackedData[baseDataOffset] = data.get(baseDim, dataIndex);
+ stackedData[1 - baseDataOffset] =
data.get(data.getCalculationInfo('stackResultDimension'), dataIndex);
+ point = coordSys.dataToPoint(stackedData) || [];
+ console.log(11111111)
Review comment:
`console.log(11111111)` should not exist.
##########
File path: src/component/axisPointer/findPointFromSeries.ts
##########
@@ -57,7 +58,20 @@ export default function (finder: {
const el = data.getItemGraphicEl(dataIndex);
const coordSys = seriesModel.coordinateSystem;
- if (seriesModel.getTooltipPosition) {
+ if (coordSys && coordSys.dataToPoint && finder.isStacked) {
Review comment:
In my opinion, `seriesModel.getTooltipPosition` should have higher
priority than this logic.
And this logic is similar logic as the `else if` branch in line 77~85., so
this code can be put there.
##########
File path: src/component/marker/MarkerModel.ts
##########
@@ -206,19 +203,16 @@ abstract class MarkerModel<Opts extends MarkerOption =
MarkerOption> extends Com
const value = this.getRawValue(dataIndex);
const formattedValue = zrUtil.isArray(value)
? zrUtil.map(value, addCommas).join(', ') : addCommas(value as
number);
- const name = data.getName(dataIndex);
+ const name = encodeHTML(data.getName(dataIndex));
Review comment:
If `name` is encoded here, then it will be added to var `html`, and then
it will be input to `concatTooltipHtml` and be encoded twice. That would be
wrong if there are some char like '<' ';' '>' in name.
##########
File path: src/component/marker/markerHelper.ts
##########
@@ -81,9 +81,9 @@ function markerTypeCalculatorWithExtent(
? data.getCalculationInfo('stackResultDimension')
: targetDataDim;
- const value = numCalculate(data, calcDataDim, markerType);
+ const value = numCalculate(data, targetDataDim, markerType);
Review comment:
Not sure why make this change. It's seems not correct for stacked chart?
##########
File path: src/chart/radar/RadarSeries.ts
##########
@@ -101,8 +101,10 @@ class RadarSeriesModel extends
SeriesModel<RadarSeriesOption> {
return encodeHTML(name === '' ? this.name : name) + '<br/>'
+ zrUtil.map(indicatorAxes, function (axis, idx) {
const val = data.get(data.mapDimension(axis.dim), dataIndex);
- return encodeHTML(axis.name + ' : ' + val);
- }).join('<br />');
+ return '<p style="margin: 8px 0 0;">'
Review comment:
Here we use `<p>` instead of the previous `<br/>`. In my opinion, may be
we should add an inline css 'display: block' to prevent the `display` of `<p>`
from being overridden by the environment.
(I know it not appropriate to do that override in most cases, but env is
harsh and we can do something to reduce the bad cases if possible.)
in my opinion, do not use <p>, in case that p is defined other css
properties like `line-height`, padding, and so on.
It's OK to use div.
##########
File path: src/component/tooltip/TooltipModel.ts
##########
@@ -63,6 +63,8 @@ export interface TooltipOption extends
CommonTooltipOption<TopLevelFormatterPara
* Only available when renderMode is html
*/
appendToBody?: boolean
+
+ order?: 'valueAsc' | 'valueDesc' | 'legendAsc' | 'legendDesc'
Review comment:
(1) Can not find the test case about this feature in `new-tooltip.html`?
(2) I think it should not be named as 'legendDesc'.
It is not the legend order if legend.data declared in an other order.
In fact it is the "series declaration order".
(3) Do we need the config that "order by series declaration but desc" ?
I've not found a use case for that yet.
And do we need the config that "order by value but inverse to the display"?
That will be a miss leading and probably never be used, I guess.
So I think here really meaningful options would be only:
+ order by series declaration (by default)
+ order by values on axis (the order should be the same as they are
displayed, that is, if `yAxis.inverse: true` is specified, the order should be
followed.
What's your opinion @pissang @Ovilia ?
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -38,6 +39,55 @@ const vendors = ['', '-webkit-', '-moz-', '-o-'];
const gCssText =
'position:absolute;display:block;border-style:solid;white-space:nowrap;z-index:9999999;';
+function mirrowPos(pos: string): string {
+ pos = pos === 'left'
+ ? 'right'
+ : pos === 'right'
+ ? 'left'
+ : pos === 'top'
+ ? 'bottom'
+ : 'top';
+ return pos;
+}
+
+function assembleArrow(
+ backgroundColor: ColorString,
+ borderColor: ZRColor,
+ arrowPosition: TooltipOption['position']
+) {
+ if (zrUtil.isObject(borderColor) && borderColor.type !== 'pattern') {
Review comment:
Probably better to directly detect the feature of "gradient color"
rather than `!== 'pattern'`?
For example: detect whether `colorStop` exist?
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -79,6 +129,7 @@ function assembleCssText(tooltipModel: Model<TooltipOption>)
{
const transitionDuration = tooltipModel.get('transitionDuration');
const backgroundColor = tooltipModel.get('backgroundColor');
+ const boxShadow = tooltipModel.get('boxShadow');
Review comment:
There is no echarts option named as `boxShadow` yet but have lots of
option named as `shadowColor` `shadowBlur` `shadowOffsetX` `shadowOffsetY`.
I think we should better follow that convention, to give users consistent
knowledge.
Especially there is another type of tooltip implemented not depends on DOM
(for mini program env). The new `option` box shadow is a CSS value, which
brings trouble to parse if we intent to adopt shadow feature in non-DOM tooltip.
Probably we can retrieve `shadowColor` `shadowBlur` `shadowOffsetX`
`shadowOffsetY` from tooltipModel and make css `box-shadow` by these values.
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -259,16 +314,24 @@ class TooltipHTMLContent {
// this.hide();
}
- show(tooltipModel: Model<TooltipOption>) {
+ show(tooltipModel: Model<TooltipOption>, nearPointColor: ZRColor) {
clearTimeout(this._hideTimeout);
const el = this.el;
const styleCoord = this._styleCoord;
+ const offset = el.offsetHeight / 2;
+ if (zrUtil.isObject(nearPointColor) && nearPointColor.type !==
'pattern') {
Review comment:
This code snippet is repeated with above code in line 58, should better
be put in an util function.
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -735,14 +782,20 @@ class TooltipView extends ComponentView {
const formatter = tooltipModel.get('formatter');
positionExpr = positionExpr || tooltipModel.get('position');
let html = defaultHtml;
+ const nearPoint = this._getNearestPoint(
+ [x, y],
+ params,
+ (tooltipModel.ecModel.getComponent('series') as
CoordinateSystemHostModel)
Review comment:
Should not get the coordinate system from the "first" series.
A chart instance can contain multiple series and the first series might do
not belong to this axis or even do not belong to this coordinate system.
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -38,6 +39,55 @@ const vendors = ['', '-webkit-', '-moz-', '-o-'];
const gCssText =
'position:absolute;display:block;border-style:solid;white-space:nowrap;z-index:9999999;';
+function mirrowPos(pos: string): string {
+ pos = pos === 'left'
+ ? 'right'
+ : pos === 'right'
+ ? 'left'
+ : pos === 'top'
+ ? 'bottom'
+ : 'top';
+ return pos;
+}
+
+function assembleArrow(
+ backgroundColor: ColorString,
+ borderColor: ZRColor,
+ arrowPosition: TooltipOption['position']
+) {
+ if (zrUtil.isObject(borderColor) && borderColor.type !== 'pattern') {
+ borderColor = borderColor.colorStops[0].color;
+ }
+ else if (zrUtil.isObject(borderColor) && (borderColor.type === 'pattern'))
{
+ borderColor = 'transparent';
+ }
+ if (typeof arrowPosition !== 'string') {
Review comment:
`zrUtil.isString(arrowPosition)` is more neat.
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -541,23 +565,47 @@ class TooltipView extends ComponentView {
renderMode
});
- if (dataParams) {
- singleParamsList.push(dataParams);
- const seriesTooltip = series.formatTooltip(
- dataIndex, true, null, renderMode as
TooltipRenderMode
- );
-
- let html;
- if (zrUtil.isObject(seriesTooltip)) {
- html = seriesTooltip.html;
- const newMarkers = seriesTooltip.markers;
- zrUtil.merge(markers, newMarkers);
- }
- else {
- html = seriesTooltip;
- }
- seriesDefaultHTML.push(html);
+ singleParamsList.push(dataParams);
+ const seriesTooltip = series.formatTooltip(
+ dataIndex, true, null, renderMode as TooltipRenderMode
+ );
+
+ let html;
+ if (zrUtil.isObject(seriesTooltip)) {
+ html = seriesTooltip.html;
+ const newMarkers = seriesTooltip.markers;
+ zrUtil.merge(markers, newMarkers);
+ }
+ else {
+ html = seriesTooltip;
}
+ dataParams.html = html;
+ });
+
+ switch (singleTooltipModel.get('order')) {
+ case 'valueAsc':
+ singleParamsList.sort(function (a, b) {
+ return a.data - b.data;
+ });
+ break;
+
+ case 'valueDesc':
+ singleParamsList.sort(function (a, b) {
+ return b.data - a.data;
+ });
+ break;
+
+ case 'legendDesc':
+ singleParamsList.reverse();
+ break;
+
+ case 'legendAsc':
+ default:
+ break;
+ }
+
+ singleParamsList.forEach(element => {
Review comment:
Use `zrUtill.each` instead.
There is no polyfill of `forEach` now.
##########
File path: src/util/format.ts
##########
@@ -66,6 +66,13 @@ export function encodeHTML(source: string): string {
});
}
+export function concatTooltipHtml(html: string, value: unknown, dontEncodeHtml
= false): string {
+ return (dontEncodeHtml ? html : encodeHTML(html))
+ + (value ? '<strong
style="float:right;margin-left:20px;color:#000;">' : '')
Review comment:
In my opinion, we should not replay on the default style of user agent.
`<strong>` may be defined as other style in different environments, that
might makes the final effect out of control.
We should better write inline style directly, and use tag like `span` that
has lower chance to be style-redefined
##########
File path: test/new-tooltip.html
##########
@@ -0,0 +1,2950 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<!DOCTYPE html>
+
+<head>
+ <meta charset="utf-8">
+ <script src="lib/esl.js"></script>
+ <script src="lib/config.js"></script>
+</head>
+
+<body>
+ <h1>Tooltip in Line Chart</h1>
+ <div id="main1" style="width: 100vw;height:40vh;margin: 0 auto;"></div>
Review comment:
The `<body>` has a `margin: 8px`, here `width: 100vw` make of div
overflow the viewport.
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -528,8 +533,27 @@ class TooltipView extends ComponentView {
zrUtil.each(item.seriesDataIndices, function (idxItem) {
const series =
ecModel.getSeriesByIndex(idxItem.seriesIndex);
+ const data = series.getData();
const dataIndex = idxItem.dataIndexInside;
+ const dims =
zrUtil.map(series.coordinateSystem.dimensions, function (coordDim) {
+ return data.mapDimension(coordDim);
+ });
const dataParams = series &&
series.getDataParams(dataIndex) as TooltipDataParams;
+ let isStacked = false;
+ const stackResultDim =
data.getCalculationInfo('stackResultDimension');
+ if (isDimensionStacked(data, dims[0])) {
+ isStacked = true;
+ dims[0] = stackResultDim;
+ }
+ if (isDimensionStacked(data, dims[1])) {
+ isStacked = true;
+ dims[1] = stackResultDim;
+ }
+ dataParams.position = findPointFromSeries({
Review comment:
`dataParams` will be exposed to users in callback.
So every properties on `dataParams` should have a clear meaning for users,
and any change in future will be a break change.
I think it's not easy to tell users what this `position`. And it might be
changed in future.
So it should better not be mounted on dataParams.
##########
File path: src/model/Series.ts
##########
@@ -526,7 +526,7 @@ class SeriesModel<Opt extends SeriesOption = SeriesOption>
extends ComponentMode
: tooltipDimLen
? formatSingleValue(retrieveRawValue(data, dataIndex,
tooltipDims[0]))
: formatSingleValue(isValueArr ? value[0] : value);
- const content = formattedValue.content;
+ const content = `<strong
style="float:right;margin-left:20px;color:#000;">${formattedValue.content}</strong>`;
Review comment:
In my opinion, we should not replay on the default style of user agent.
`<strong>` may be defined as other style in different environments, that
might makes the final effect out of control.
We should better write inline style directly, and use tag like `span` that
has lower chance to be style-redefined.
${formattedValue.content} may be contain some html "block" tags (for example
in candlestick and boxplot)? not good to encapsulated by a tag inline
`<strong>` ?
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -541,23 +565,47 @@ class TooltipView extends ComponentView {
renderMode
});
- if (dataParams) {
- singleParamsList.push(dataParams);
- const seriesTooltip = series.formatTooltip(
- dataIndex, true, null, renderMode as
TooltipRenderMode
- );
-
- let html;
- if (zrUtil.isObject(seriesTooltip)) {
- html = seriesTooltip.html;
- const newMarkers = seriesTooltip.markers;
- zrUtil.merge(markers, newMarkers);
- }
- else {
- html = seriesTooltip;
- }
- seriesDefaultHTML.push(html);
+ singleParamsList.push(dataParams);
+ const seriesTooltip = series.formatTooltip(
+ dataIndex, true, null, renderMode as TooltipRenderMode
+ );
+
+ let html;
+ if (zrUtil.isObject(seriesTooltip)) {
+ html = seriesTooltip.html;
+ const newMarkers = seriesTooltip.markers;
+ zrUtil.merge(markers, newMarkers);
+ }
+ else {
+ html = seriesTooltip;
}
+ dataParams.html = html;
+ });
+
+ switch (singleTooltipModel.get('order')) {
+ case 'valueAsc':
+ singleParamsList.sort(function (a, b) {
+ return a.data - b.data;
Review comment:
`data` might be an array, can not be minus directly.
I suggest change the ts type of `CallbackDataParams['data']` and
`CallbackDataParams['value']` from `any` to `unknown`. `any` might lead us to
use it but not check what it is.
And both `data` and `value` property here is from "raw data". Values in "raw
data" is not parsed yet.
For example, if user set
```js
option = {
xAxis: { type: 'time' },
yAxis: {},
series: [{
type: 'line',
data: [
['2012-2-12', 9988],
['2012-2-13', 10021],
]
}]
};
```
The prop `data` would be `['2012-2-12', 9988]` here and prop `value` would
be string `'2012-2-12'`.
That date string should not be used to sort.
If really need to sort, we should better user the parsed data, which can be
retrieved from like
```ts
const data = series.getData()
const axisDimension = ...
const dataDimension = data.mapDimension(axisDimension);
const val = data.get(dataDimension);
// sort by that val
```
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -752,12 +805,50 @@ class TooltipView extends ComponentView {
html = formatter(params, asyncTicket, callback);
}
- tooltipContent.setContent(html, markers, tooltipModel);
- tooltipContent.show(tooltipModel);
-
this._updatePosition(
tooltipModel, positionExpr, x, y, tooltipContent, params, el
);
+ tooltipContent.setContent(html, markers, tooltipModel,
nearPoint.color, positionExpr);
+ tooltipContent.show(tooltipModel, nearPoint.color);
+
+ }
+
+ _getNearestPoint(
+ point: number[],
+ tooltipDataParams: TooltipDataParams | TooltipDataParams[],
+ coord?: CoordinateSystem
+ ): {
+ x: number;
+ y: number;
+ color: ZRColor;
+ } {
+ let dim = '';
+ if (coord && coord.type === 'cartesian2d') {
+ dim = coord.getBaseAxis().dim;
+ }
+ if (!zrUtil.isArray(tooltipDataParams)) {
+ if (!tooltipDataParams.position) {
+ return {
+ x: point[0],
+ y: point[1],
+ color: tooltipDataParams.color ||
tooltipDataParams.borderColor
+ };
+ }
+ return {
+ x: point[0],
+ y: tooltipDataParams.position[1],
+ color: tooltipDataParams.color || tooltipDataParams.borderColor
+ };
+ }
+
+ const posIndex = +(dim === 'x');
+ const distanceArr = tooltipDataParams.map(params =>
Math.abs(params.position[posIndex] - point[posIndex]));
+ const index = distanceArr.indexOf(Math.min(...distanceArr));
+ return {
+ x: tooltipDataParams[index]?.position[0] || point[0],
Review comment:
There might be no that `position` prop. `_showTooltipContent` is called
in two places. See line 746.
And see the comment above, position info should better not put in this
`tooltipDataParams`.
##########
File path: src/model/Series.ts
##########
@@ -545,22 +545,19 @@ class SeriesModel<Opt extends SeriesOption =
SeriesOption> extends ComponentMode
seriesName = '';
}
seriesName = seriesName
- ? encodeHTML(seriesName) + (!multipleSeries ? newLine : ': ')
+ ? encodeHTML(seriesName) + (!multipleSeries ? newLine : ' ')
: '';
colorStr = typeof colorEl === 'string' ? colorEl : colorEl.content;
const html = !multipleSeries
- ? seriesName + colorStr
+ ? seriesName + (seriesName ? '<br/>' : '') + '<p style="margin:
8px 0 0;">' + colorStr
Review comment:
in my opinion, do not use `<p>`, in case that p is defined with other
css properties like `line-height`, padding, and so on.
It's OK to use div.
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -752,12 +805,50 @@ class TooltipView extends ComponentView {
html = formatter(params, asyncTicket, callback);
}
- tooltipContent.setContent(html, markers, tooltipModel);
- tooltipContent.show(tooltipModel);
-
this._updatePosition(
tooltipModel, positionExpr, x, y, tooltipContent, params, el
);
+ tooltipContent.setContent(html, markers, tooltipModel,
nearPoint.color, positionExpr);
+ tooltipContent.show(tooltipModel, nearPoint.color);
+
+ }
+
+ _getNearestPoint(
+ point: number[],
+ tooltipDataParams: TooltipDataParams | TooltipDataParams[],
+ coord?: CoordinateSystem
+ ): {
+ x: number;
+ y: number;
+ color: ZRColor;
+ } {
+ let dim = '';
+ if (coord && coord.type === 'cartesian2d') {
+ dim = coord.getBaseAxis().dim;
+ }
+ if (!zrUtil.isArray(tooltipDataParams)) {
+ if (!tooltipDataParams.position) {
+ return {
+ x: point[0],
+ y: point[1],
+ color: tooltipDataParams.color ||
tooltipDataParams.borderColor
+ };
+ }
+ return {
+ x: point[0],
+ y: tooltipDataParams.position[1],
+ color: tooltipDataParams.color || tooltipDataParams.borderColor
+ };
+ }
+
+ const posIndex = +(dim === 'x');
+ const distanceArr = tooltipDataParams.map(params =>
Math.abs(params.position[posIndex] - point[posIndex]));
+ const index = distanceArr.indexOf(Math.min(...distanceArr));
+ return {
+ x: tooltipDataParams[index]?.position[0] || point[0],
+ y: tooltipDataParams[index]?.position[1] || point[1],
+ color: tooltipDataParams[index]?.color ||
tooltipDataParams[index]?.borderColor
Review comment:
Should better have a default color ?
##########
File path: src/util/types.ts
##########
@@ -1034,6 +1036,7 @@ export interface CommonTooltipOption<FormatterParams> {
borderColor?: ColorString
borderRadius?: number
borderWidth?: number
+ boxShadow?: ColorString
Review comment:
As commented above, should not be named as `boxShadow` (and the ts type
of css boxShadow is not `ColorString`).
Should better be:
shadowBlur
shadowColor
shadowOffsetX
shadowOffsetY
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -752,12 +805,50 @@ class TooltipView extends ComponentView {
html = formatter(params, asyncTicket, callback);
}
- tooltipContent.setContent(html, markers, tooltipModel);
- tooltipContent.show(tooltipModel);
-
this._updatePosition(
tooltipModel, positionExpr, x, y, tooltipContent, params, el
);
+ tooltipContent.setContent(html, markers, tooltipModel,
nearPoint.color, positionExpr);
+ tooltipContent.show(tooltipModel, nearPoint.color);
+
+ }
+
+ _getNearestPoint(
+ point: number[],
+ tooltipDataParams: TooltipDataParams | TooltipDataParams[],
+ coord?: CoordinateSystem
+ ): {
+ x: number;
+ y: number;
+ color: ZRColor;
+ } {
+ let dim = '';
+ if (coord && coord.type === 'cartesian2d') {
+ dim = coord.getBaseAxis().dim;
+ }
+ if (!zrUtil.isArray(tooltipDataParams)) {
+ if (!tooltipDataParams.position) {
+ return {
+ x: point[0],
+ y: point[1],
+ color: tooltipDataParams.color ||
tooltipDataParams.borderColor
+ };
+ }
+ return {
+ x: point[0],
+ y: tooltipDataParams.position[1],
Review comment:
Do not understand this logic. Why x y are traded differently.
But in fact, the return {x, y} from `_getNearestPoint` are never used?
So probably {x, y} related logic here can be removed?
And this method should only be in charge of getting color ?
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -752,12 +805,50 @@ class TooltipView extends ComponentView {
html = formatter(params, asyncTicket, callback);
}
- tooltipContent.setContent(html, markers, tooltipModel);
- tooltipContent.show(tooltipModel);
-
this._updatePosition(
tooltipModel, positionExpr, x, y, tooltipContent, params, el
);
+ tooltipContent.setContent(html, markers, tooltipModel,
nearPoint.color, positionExpr);
+ tooltipContent.show(tooltipModel, nearPoint.color);
+
+ }
+
+ _getNearestPoint(
+ point: number[],
+ tooltipDataParams: TooltipDataParams | TooltipDataParams[],
+ coord?: CoordinateSystem
+ ): {
+ x: number;
+ y: number;
+ color: ZRColor;
+ } {
+ let dim = '';
+ if (coord && coord.type === 'cartesian2d') {
+ dim = coord.getBaseAxis().dim;
+ }
+ if (!zrUtil.isArray(tooltipDataParams)) {
+ if (!tooltipDataParams.position) {
+ return {
+ x: point[0],
+ y: point[1],
+ color: tooltipDataParams.color ||
tooltipDataParams.borderColor
+ };
+ }
+ return {
+ x: point[0],
+ y: tooltipDataParams.position[1],
+ color: tooltipDataParams.color || tooltipDataParams.borderColor
+ };
+ }
+
+ const posIndex = +(dim === 'x');
Review comment:
Not a good way to determine axis dimension. only suitable for cartesian.
We could use Euclidean distance to avoid that axis dimension determine.
##########
File path: src/util/types.ts
##########
@@ -113,6 +113,8 @@ export interface ECElement extends Element {
hoverState?: 0 | 1 | 2;
selected?: boolean;
+ style?: Dictionary<any>;
Review comment:
Use `unknown` instead of `any`. Avoid `any` spreads.
##########
File path: src/component/tooltip/TooltipView.ts
##########
@@ -541,23 +565,47 @@ class TooltipView extends ComponentView {
renderMode
});
- if (dataParams) {
- singleParamsList.push(dataParams);
- const seriesTooltip = series.formatTooltip(
- dataIndex, true, null, renderMode as
TooltipRenderMode
- );
-
- let html;
- if (zrUtil.isObject(seriesTooltip)) {
- html = seriesTooltip.html;
- const newMarkers = seriesTooltip.markers;
- zrUtil.merge(markers, newMarkers);
- }
- else {
- html = seriesTooltip;
- }
- seriesDefaultHTML.push(html);
+ singleParamsList.push(dataParams);
+ const seriesTooltip = series.formatTooltip(
+ dataIndex, true, null, renderMode as TooltipRenderMode
+ );
+
+ let html;
+ if (zrUtil.isObject(seriesTooltip)) {
+ html = seriesTooltip.html;
+ const newMarkers = seriesTooltip.markers;
+ zrUtil.merge(markers, newMarkers);
+ }
+ else {
+ html = seriesTooltip;
}
+ dataParams.html = html;
Review comment:
dataParams will be exposed to users in some callback.
Is it intentionally expose `html` to users in callback?
I think the html is possible to be changed in one day if some new
requirements come,
so it should better not to be exposed to users unless really necessary ?
----------------------------------------------------------------
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]
