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: export registerSymbol #6381

Merged
merged 7 commits into from
Jul 26, 2024
Merged

feat: export registerSymbol #6381

merged 7 commits into from
Jul 26, 2024

Conversation

wangxingkang
Copy link
Contributor

@wangxingkang wangxingkang commented Jul 23, 2024

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

@pearmini
Copy link
Member

@wangxingkang 非常感谢贡献!

可以参考这个 PR #6379 去添加一个测试案例吗?同时也在官网案例中添加一个例子,以及文档,让更多人看到你实现的新 feature!

@wangxingkang
Copy link
Contributor Author

好的 我加一下

@pearmini
Copy link
Member

pearmini commented Jul 25, 2024

@wangxingkang

这个地方我想了一下,应该不能把 registerSymbol(define) 这个函数暴露出去,因为现在已经有一个 register(namespace, define) 方法用于扩展能力了。所以更好的做法是对 register 进行一下增强:

import {register} from '@antv/g2';

register('symbol.custom', () => {});

实现可以参考如下:

// src/api/library.ts

import { G2ComponentNamespaces, G2Component } from '../runtime';
import { registerSymbol, SymbolFactor } from '../utils/marker';

export const library = {};

// @todo Warn if override existing key.
export function register(
  key: `${G2ComponentNamespaces | 'symbol'}.${any}`
  component: G2Component | SymbolFactor,
) {
  if (key.startsWith('symbol.'))
    registerSymbol(key.split('.').pop(), component as SymbolFactor);
  else Object.assign(library, { [key]: component });
}

后面更好的做法是把 symbol 作为一个单独的类型的可视化组件。(暂时不考虑)

@wangxingkang
Copy link
Contributor Author

好的 我参考下

@wangxingkang
Copy link
Contributor Author

集成到 register 确实更优雅

@wangxingkang
Copy link
Contributor Author

@pearmini
Copy link
Member

@wangxingkang 嗯嗯,可以滴

@wangxingkang
Copy link
Contributor Author

@pearmini 帮忙看看测试用例写的是否合适 我把文档补一下,顺便看下标题 【自定义符号(Symbol)】是否合适

@pearmini
Copy link
Member

pearmini commented Jul 25, 2024

@wangxingkang

测试代码内容没有问题,但是位置可能需要改一下!🎉

因为这个 register symbol 这个能力是只和静态渲染有关嘛,所以放在截图测试里会好一点。可以在 __tests__/plots/static 里面新建一个文件 mock-interval-legend-marker.ts,然后输入如下内容:

import { G2Spec, register, SymbolFactor } from '../../../src';

export function mockIntervalLegendMarker(): G2Spec {
  const triangle: SymbolFactor = Object.assign(
    (x, y, r) => {
      const diffY = r * Math.sin((1 / 3) * Math.PI);
      return [
        ['M', x - r, y + diffY],
        ['L', x, y - diffY],
        ['L', x + r, y + diffY],
        ['Z'],
      ];
    },
    { style: ['fill'] },
  );

  register('symbol.customTriangle', triangle);

  return {
    type: 'interval',
    data: [
      { name: 'London', 月份: 'Jan.', 月均降雨量: 18.9 },
      { name: 'London', 月份: 'Feb.', 月均降雨量: 28.8 },
      { name: 'London', 月份: 'Mar.', 月均降雨量: 39.3 },
      { name: 'London', 月份: 'Apr.', 月均降雨量: 81.4 },
      { name: 'London', 月份: 'May', 月均降雨量: 47 },
      { name: 'London', 月份: 'Jun.', 月均降雨量: 20.3 },
      { name: 'London', 月份: 'Jul.', 月均降雨量: 24 },
      { name: 'London', 月份: 'Aug.', 月均降雨量: 35.6 },
      { name: 'Berlin', 月份: 'Jan.', 月均降雨量: 12.4 },
      { name: 'Berlin', 月份: 'Feb.', 月均降雨量: 23.2 },
      { name: 'Berlin', 月份: 'Mar.', 月均降雨量: 34.5 },
      { name: 'Berlin', 月份: 'Apr.', 月均降雨量: 99.7 },
      { name: 'Berlin', 月份: 'May', 月均降雨量: 52.6 },
      { name: 'Berlin', 月份: 'Jun.', 月均降雨量: 35.5 },
      { name: 'Berlin', 月份: 'Jul.', 月均降雨量: 37.4 },
      { name: 'Berlin', 月份: 'Aug.', 月均降雨量: 42.4 },
    ],
    encode: {
      x: '月份',
      y: '月均降雨量',
      color: 'name',
    },
    transform: [{ type: 'stackY' }],
    legend: {
      color: {
        itemMarker: 'customTriangle',
      },
    },
  };
}

__tests__/plots/static/index.ts 里面导出:

export { mockIntervalLegendMarker } from './mock-interval-legend-marker.';

然后运行 npm run test 更新一下截图。

@pearmini pearmini self-requested a review July 25, 2024 08:55
@wangxingkang
Copy link
Contributor Author

@pearmini 测试相关已修改 文档相关有个疑问 这个文档是和 自定义图例(Legend)同级 还是放在 自定义图例(Legend)下面

@pearmini
Copy link
Member

pearmini commented Jul 25, 2024

@wangxingkang

测试相关已修改

应该还需要运行 npm run test 生成一下截图,然后提交,我看 diff 里面没有?

文档相关有个疑问 这个文档是和 自定义图例(Legend)同级 还是放在 自定义图例(Legend)下面

同级好一点?毕竟是两种不同的场景?

@wangxingkang
Copy link
Contributor Author

好的 我本地 run test 报错 我找找原因

@wangxingkang
Copy link
Contributor Author

@pearmini 全部搞定了,文档麻烦看下,有没有描述不当的地方

@coveralls
Copy link

Pull Request Test Coverage Report for Build 10092208461

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 27 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.03%) to 86.691%

Files with Coverage Reduction New Missed Lines %
src/interaction/legendFilter.ts 1 91.89%
src/transform/filter.ts 3 81.54%
src/runtime/plot.ts 5 97.93%
src/interaction/tooltip.ts 7 92.72%
src/runtime/scale.ts 11 93.48%
Totals Coverage Status
Change from base Build 10053542501: 0.03%
Covered Lines: 10556
Relevant Lines: 11801

💛 - Coveralls

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.

@wangxingkang 这个 PR 看上去非常棒了!感谢贡献 😊

@pearmini pearmini linked an issue Jul 26, 2024 that may be closed by this pull request
@pearmini pearmini merged commit 38b6711 into antvis:v5 Jul 26, 2024
2 checks passed
@pearmini
Copy link
Member

pearmini commented Jul 26, 2024

@wangxingkang

对了,如果有空的话还可以再提一个 PR,在官网加一个例子 site/examples/component/legend/demo。可以看看能不能解决这个 issue 提到的问题:#6075

如果可以的话,可以尝试添加一下。截图的话在 PR 里发给我,我帮你上传,然后给你一个链接,你更新到 PR 里面就好。

@wangxingkang
Copy link
Contributor Author

@pearmini 好的

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.

【V5】legend itemMarker 的内置类型是否可以扩展?
3 participants