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

feat: add default format to tooltip title #5684 #5748

Merged
merged 1 commit into from
Nov 9, 2023
Merged

feat: add default format to tooltip title #5684 #5748

merged 1 commit into from
Nov 9, 2023

Conversation

BENcorry
Copy link
Contributor

@BENcorry BENcorry commented Nov 6, 2023

Checklist
  • npm test passes
  • benchmarks are included
  • commit message follows commit guidelines
  • documents are updated
Description of change
解决问题

#5684 当图表中 encode 的 x 为日期字段的时候,tooltip title 会默认显示 x 轴的数据,对于日期的默认格式化效果不是很友好。

解决办法
  1. 先判断当前的tooltip title 是否有用户自定义的处理,如果有,对title不做任何处理
  2. 如果没有用户自定义处理,对日期格式进行动态字符处理
DEMO
  1. 存在自定义处理
    image
import { G2Spec } from '../../../src';
import { seriesTooltipSteps } from './utils';

export function aaplLine(): G2Spec {
  return {
    type: 'view',
    children: [
      {
        type: 'line',
        data: {
          type: 'fetch',
          value: 'data/aapl.csv',
        },
        encode: {
          x: 'date',
          y: 'close',
        },
        tooltip: {
          title: (d) => new Date(d.date).toUTCString(),
        },
      },
    ],
  };
}

aaplLine.steps = seriesTooltipSteps([200, 300]);
  1. 移除自定义处理
    image
import { G2Spec } from '../../../src';
import { seriesTooltipSteps } from './utils';

export function aaplLine(): G2Spec {
  return {
    type: 'view',
    children: [
      {
        type: 'line',
        data: {
          type: 'fetch',
          value: 'data/aapl.csv',
        },
        encode: {
          x: 'date',
          y: 'close',
        },
        // tooltip: {
        //   title: (d) => new Date(d.date).toUTCString(),
        // },
      },
    ],
  };
}

aaplLine.steps = seriesTooltipSteps([200, 300]);

@pearmini
Copy link
Member

pearmini commented Nov 6, 2023

这里的测试快照可以先不更新,现在的环境在不同机器上有差异,等我们解决了更新即可。这个 PR 只需要包含代码更新就行。

@BENcorry
Copy link
Contributor Author

BENcorry commented Nov 6, 2023

这里的测试快照可以先不更新,现在的环境在不同机器上有差异,等我们解决了更新即可。这个 PR 只需要包含代码更新就行。

OK, 已更新

@pearmini
Copy link
Member

pearmini commented Nov 7, 2023

在 PR 描述里面更新一些 API 的使用方式(通过一些案例体现),同时解释一下实现思路?参考这个 PR 的描述:#5751

@BENcorry
Copy link
Contributor Author

BENcorry commented Nov 7, 2023

在 PR 描述里面更新一些 API 的使用方式(通过一些案例体现),同时解释一下实现思路?参考这个 PR 的描述:#5751

已更新~

Copy link
Member

@pearmini pearmini left a comment

Choose a reason for hiding this comment

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

  • format 函数挺好!
  • 这里不建议在 tooltip 交互里面做 format,这个在提取 title 数据的过程中比较好,相关的代码在这里 https://github.com/antvis/G2/blob/v5/src/runtime/transform.ts#L187 ,这里可能需要根据数据类型 value1 去设置默认的 formatter
  • 参考 __tests__/plots/tooltip/aapl-line.ts 添加一个测试,CI 报错可以不管

@BENcorry BENcorry closed this Nov 8, 2023
@BENcorry BENcorry reopened this Nov 8, 2023
@BENcorry
Copy link
Contributor Author

BENcorry commented Nov 8, 2023

  • format 函数挺好!
  • 这里不建议在 tooltip 交互里面做 format,这个在提取 title 数据的过程中比较好,相关的代码在这里 https://github.com/antvis/G2/blob/v5/src/runtime/transform.ts#L187 ,这里可能需要根据数据类型 value1 去设置默认的 formatter
  • 参考 __tests__/plots/tooltip/aapl-line.ts 添加一个测试,CI 报错可以不管
  1. title的处理过程中针对于无回调处理的title做了处理
  2. 新增测试文件__tests__/plots/tooltip/aapl-line-date-default-format.ts

Copy link
Member

@pearmini pearmini left a comment

Choose a reason for hiding this comment

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

有两个地方需要改一下:

  • 不需要去处理时间字符串,如果是一个字符串就返回字符串,只有当值是 Date 对象的时候再处理。这里的部分原因是 Date.parse 在不同浏览器的兼容性不同。
  • 不应该根据 title 的类型(数组?回调?对象)去判断是否格式化时间,而是当 value 是 Date 的时候统一处理,相当于替换默认的 Date.toString 函数,因为 Date 最后展示出来都是字符串。

类似所有情况,作如下的处理:

if (typeof item === 'function') {
  const values = [];
  for (const i of I) {
    const v = item(data[i], i, data, encode);
    const v1 = v instanceOf Date ? dynamicFormatDateTime(v) : v ; // 👈
    if (isStrictObject(v)) values[i] = v1;
    else values[i] = { value: v1 };
  }
  return values;
}

@BENcorry
Copy link
Contributor Author

BENcorry commented Nov 8, 2023

有两个地方需要改一下:

  • 不需要去处理时间字符串,如果是一个字符串就返回字符串,只有当值是 Date 对象的时候再处理。这里的部分原因是 Date.parse 在不同浏览器的兼容性不同。
  • 不应该根据 title 的类型(数组?回调?对象)去判断是否格式化时间,而是当 value 是 Date 的时候统一处理,相当于替换默认的 Date.toString 函数,因为 Date 最后展示出来都是字符串。

类似所有情况,作如下的处理:

if (typeof item === 'function') {
  const values = [];
  for (const i of I) {
    const v = item(data[i], i, data, encode);
    const v1 = v instanceOf Date ? dynamicFormatDateTime(v) : v ; // 👈
    if (isStrictObject(v)) values[i] = v1;
    else values[i] = { value: v1 };
  }
  return values;
}

有几个小问题:

  1. 当tooltip不做处理的时候,title到valueof的时候是一个列表,可能走不到处理逻辑
  2. 通过maybeTitle在做transform的时候直接处理会不会更好一点,这个时候Date会应为join直接转化为String

Copy link
Member

@pearmini pearmini left a comment

Choose a reason for hiding this comment

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

这样更简洁!

@pearmini pearmini merged commit b7e9055 into antvis:v5 Nov 9, 2023
2 of 3 checks passed
Exlau pushed a commit to Exlau/G2 that referenced this pull request Nov 10, 2023
Co-authored-by: ben.wen <ben.wen@narwal.com>
@hustcc
Copy link
Member

hustcc commented Nov 13, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants