Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[5.0] New tooltip design #12947

Merged
merged 79 commits into from Jul 30, 2020
Merged

[5.0] New tooltip design #12947

merged 79 commits into from Jul 30, 2020

Conversation

susiwen8
Copy link
Contributor

@susiwen8 susiwen8 commented Jul 9, 2020 •
edited

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

Progress

  • New style: background color, box shadow, border color, font weight
  • tooltip order: add new order option for order, provided values are valueAsc | valueDesc | legendDesc | legendAsc. Default is legendAsc.
  • border color is the color of current series.
  • tooltip arrow points to current series(trigger is item).
  • tooltip border color for stacked line chart
  • tooltip border color with datazoom
  • tooltip border color with visualMap
  • line
  • bar
  • tree
  • tree map
  • sankey
  • radar
  • graph
  • pie
  • themeriver
  • map
  • scatter

What does this PR do?

New tooltip style for 5.0

Tooltip arrow

When trigger was item and position is top | bottom | left | right, Tooltip will show arrow and it points to current series
Screen Shot 2020-07-20 at 18 20 41
Screen Shot 2020-07-20 at 18 21 00
Screen Shot 2020-07-20 at 18 21 12
Screen Shot 2020-07-20 at 18 21 22

Tooltip border color

trigger was item, border color will be the color of this series
trigger was axis, border color depends on whichever closer to cursor.
Screen Shot 2020-07-20 at 18 25 28
Screen Shot 2020-07-20 at 18 25 32

Fixed issues

Close #12929

Details

Before: What was the problem?

After: How is it fixed in this PR?

Usage

Are there any API changes?

  • The API has been changed.

Related test cases or examples to use the new APIs

NA.

Others

Merging options

  • Please squash the commits into a single one when merge.

Other information

TODO

  1. position in rich mode
  2. first time showing tooltip, tooltip rise from bottom

@echarts-bot
Copy link

echarts-bot bot commented Jul 9, 2020

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 9, 2020
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 12, 2020
pissang
pissang requested changes Jul 28, 2020
View reviewed changes
src/component/tooltip/TooltipView.ts
}

const distanceArr = tooltipDataParams.map(params => {
let dim = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use zrUtil.map instead of native map

src/component/tooltip/TooltipView.ts
return distance;
});
const index = distanceArr.indexOf(Math.min(...distanceArr));
return {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use zrUtil.indexOf instead of native indexOf.

src/component/tooltip/TooltipView.ts
@@ -985,7 +1077,8 @@ function calcTooltipPosition(
): [number, number] {
const domWidth = contentSize[0];
const domHeight = contentSize[1];
const gap = 5;
const gap = 10;
const offset = 5;
let x = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't get the difference between offset and gap

src/component/marker/markerHelper.ts Outdated
@@ -81,9 +81,9 @@ function markerTypeCalculatorWithExtent(
? data.getCalculationInfo('stackResultDimension')
: targetDataDim;

const value = numCalculate(data, calcDataDim, markerType);
const value = numCalculate(data, targetDataDim, markerType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to put it in a separate PR and have further discussion about this change.

@pissang
Copy link
Contributor

pissang commented Jul 28, 2020 •
edited

Also I found the display is wrong in renderMode: 'rich'

image

And in the candlestick chart. The newline seems broken.

image

Perhaps it's caused by this change https://github.com/apache/incubator-echarts/pull/12947/files#diff-a99622711c93ad25fc31052794ad226fL142

@pissang pissang changed the title New tooltip design [5.0] New tooltip design Jul 29, 2020
@Ovilia Ovilia mentioned this pull request Jul 29, 2020
Merged
5 tasks
@susiwen8
Copy link
Contributor Author

Also I found the display is wrong in renderMode: 'rich'

image

renderMode doesn't have rich but richText, use richText there is no problem

@susiwen8
Copy link
Contributor Author

@pissang Tooltip is ready, please review it

@pissang pissang merged commit 95534a4 into apache:next Jul 30, 2020
@echarts-bot
Copy link

echarts-bot bot commented Jul 30, 2020

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

@susiwen8 susiwen8 deleted the tooltip branch July 30, 2020 02:28
@pissang pissang mentioned this pull request Aug 4, 2020
Merged
5 tasks
@100pah 100pah mentioned this pull request Sep 7, 2020
Merged
3 tasks
@susiwen8 susiwen8 mentioned this pull request Oct 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pissang pissang pissang requested changes

@100pah 100pah Awaiting requested review from 100pah

Assignees
No one assigned
Labels
PR: author is committer size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@susiwen8 @pissang @Ovilia @100pah @plainheart