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: update & getDefaultOptions bugs #1850

Merged
merged 6 commits into from
Nov 4, 2020
Merged

fix: update & getDefaultOptions bugs #1850

merged 6 commits into from
Nov 4, 2020

Conversation

lxfu1
Copy link
Member

@lxfu1 lxfu1 commented Nov 4, 2020

问题描述

  • 由于当前 getDefaultOptions 依赖于 options ,导致更新的时候需要重复调用 getDefaultOptions,用户是增量更新,调用 getDefaultOptions 时的形参需要 merge 当前 options (this.options),当前 getDefaultOptions 大多数使用默认值,会存在 bug,修复成本比较高,因此决定 getDefaultOptions 不再依赖 options,在 adaptor 里面处理。

示例

  /**
   * 更新配置
   * @param options
   */
  public update(options: O) {
    // options 更新是全量更新,这里和构造函数中一会加上图表的默认选项
    // options => {data: []}
    // this.options => { innerRadius: 0.3 }
    this.options = deepAssign({}, this.options, this.getDefaultOptions({ ...this.options, ...options }), options);
    // this.options => {innerRadius: 0, data: []}
    this.render();
  }

  protected getDefaultOptions(options: Options) {
    return deepAssign({}, super.getDefaultOptions(), {
      innerRadius: 0,
    });
  }

修复方案

二次更新不再调用 getDefaultOptions

  /**
   * 更新配置
   * @param options
   */
  public update(options: O) {
    this.options = deepAssign({}, this.options, options);
    this.render();
  }

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2020

😭 Deploy PR Preview bc4f7c8 failed. Build logs

🤖 By surge-preview

@hustcc hustcc changed the title fix: update fix: update & getDefaultOptions bugs Nov 4, 2020
@hustcc
Copy link
Member

hustcc commented Nov 4, 2020

后面 getDefaultOptions 为静态方法,不依赖 options 配置,依赖 options 的默认配置到 Adaptor 中处理。

hustcc and others added 2 commits November 4, 2020 13:57
…1851)

* fix(update): line, area update bugs

* fix(update): bidirectional-bar

* fix(update): bar, column
* fix: 去除 defaultOption 依赖

* fix: 移动目录

* fix: path

* fix: 修复引用问题
@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2020

This pull request introduces 2 alerts when merging 5650d63 into dd11880 - view on LGTM.com

new alerts:

  • 1 for Useless conditional
  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2020

This pull request introduces 2 alerts when merging 67ef799 into a59301f - view on LGTM.com

new alerts:

  • 1 for Useless conditional
  • 1 for Unused variable, import, function or class

src/utils/data.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2020

This pull request introduces 2 alerts when merging c705b7b into a59301f - view on LGTM.com

new alerts:

  • 1 for Useless conditional
  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Nov 4, 2020

This pull request introduces 2 alerts when merging 8d7ffb5 into a59301f - view on LGTM.com

new alerts:

  • 1 for Useless conditional
  • 1 for Unused variable, import, function or class

@hustcc hustcc merged commit bc4f7c8 into master Nov 4, 2020
@hustcc hustcc deleted the fix/update branch November 4, 2020 10:06
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