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

fix: #13242

Merged
merged 2 commits into from
Sep 7, 2020
Merged

fix: #13242

merged 2 commits into from
Sep 7, 2020

Conversation

100pah
Copy link
Member

@100pah 100pah commented Sep 7, 2020

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

fix:

  • Fix some buggy display result in tooltip, like 2 value axis scatter, add add test cases for them.
  • Buggy results in tooltip.renderModel: 'richText' for each cases. And add test cases for them.
  • Add styles to renderMode: 'richText', make them the same as renderMode: 'html'.
  • Make tooltip.confine default true if tooltip.renderMode is richText. And fix the positioning of it to avoid to overflow the container, which will be cut.
  • Fix the tooltip.formatter callback in renderMode: 'richText', previously the concatenating rich text by params.marker.content does not work.
    formatter: function (params) { 
         return params.map(param => param.marker.content + param.name + param.value);
    }
  • Refactor the implementation of Series['formatTooltip']:
    Add a abstract layer named TooltipMarkup (see tooltipMarkup.ts) to describe the tooltip layout requirements for each series,
    instead of lots of messy code to concatenate html and richText in each series.
    TooltipMarkup is responsible for generating html or richText finally.
    Add TooltipMarkup is also in charge of sort the tooltip by valueAsc, valueDesc, seriesAsc, seriesDesc.
    With the help of it, a typical tooltip description can be:
    formatTooltip(dataIndex: number) {
        ......
        return createTooltipMarkup('section', {
            header: xxx,
            sortBlocks: true,
            blocks: zrUtil.map(indicatorAxes, axis => {
                const val = data.get(data.mapDimension(axis.dim), dataIndex);
                return createTooltipMarkup('nameValue', {
                    markerType: 'subItem',
                    markerColor: ...,
                    name: ...,
                    value: val,
                    sortParam: val
                });
            })
        });
    }

PENDING

Do we need to expose API (like tooltipMarkup.ts#TooltipMarkupStyleCreator) to tooltip.formatter callback, by which users can add richText style themselves. At present users is not able to do that.
But if we expose that kind of API, there is an issue:
should we use ZRender style (use fill to describe text color)
or ECharts label style (use color to describe text color).

Like

formatter: function (params) { 
    if (params.renderMode === 'richText') {
        const valueStyle = { align: 'right', fontWeight: 900, fill: '#blue' };
        return params.map(param => param.marker.content + param.name + params.wrapRichTextStyle(param.value));
    }
    ...
}

Other info

test cases: test/new-tooltip.html
related PR: #12947

(1) Fix some buggy display result in tooltip, like 2 value axis scatter, add add test cases for them.
(2) Buggy results in tooltip.renderModel: 'richText' for each cases. And add test cases for them.
(3) Add styles to renderMode: 'richText', make them the same as renderMode: 'html'.
(4) Make `tooltip.confine` default `true` if `tooltip.renderMode` is `richText`. And fix the positioning of it to avoid to overflow the container, which will be cut.
(5) Fix the `tooltip.formatter` callback in `renderMode: 'richText'`, previously the concatenating rich text by `params.marker.content` does not work.
(6) Refactor the implementation of `Series['formatTooltip']`:
Add a abstract layer named `TooltipMarkup` to describe the tooltip layout requirement for each series,
instead of lots of messy code to concatenate `html` and `richText` in each series.
`TooltipMarkup` is responsible for generating `html` or `richText` finally.
Add `TooltipMarkup` is also in charge of sort the tooltip by `valueAsc`, `valueDesc`, `seriesAsc`, `seriesDesc`.

[TEST_CASES]: test/new-tooltip.html
[PENDING]: Do we need to expose API (like `tooltipMarkup.ts#TooltipMarkupStyleCreator`) to `tooltip.formatter` callback, by which users can add richText style themselves. At present users is not able to do that.
But if we expose that kind of API, there is an issue:
should we use ZRender style (use `fill` to describe text color)
or ECharts label style (use `color` to describe text color).
@echarts-bot
Copy link

echarts-bot bot commented Sep 7, 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.

The pull request is marked to be PR: author is committer because you are a committer of this project.

@100pah 100pah merged commit 85a47ef into next Sep 7, 2020
@echarts-bot
Copy link

echarts-bot bot commented Sep 7, 2020

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

@pissang pissang deleted the new-tooltip2 branch October 11, 2021 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants