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: 增加显示目标值label #1599 #1605

Merged
merged 3 commits into from
Sep 24, 2020
Merged

Conversation

arcsin1
Copy link
Contributor

@arcsin1 arcsin1 commented Sep 20, 2020

@auto-add-label auto-add-label bot added the enhancement New feature or request label Sep 20, 2020
@hustcc
Copy link
Member

hustcc commented Sep 21, 2020

为啥 target label 要单独一个配置,而不是直接和 label 配置统一一起的?

@arcsin1
Copy link
Contributor Author

arcsin1 commented Sep 21, 2020

为啥 target label 要单独一个配置,而不是直接和 label 配置统一一起的?

  1. 主要是多图的label 不是共用的,只能给一个图,默认是给measure的,那我只能在加一个label给到target
  2. 目标值的显示其实必要性:只能算一般,没有measure的值那么强烈!主要v1版的tooltip不友好,现在tooltip显示很好了
  3. 为了图好看 ,视觉好些,其实现实target的label必要性真不大,在有好的tooltip上

src/plots/bullet/types.ts Outdated Show resolved Hide resolved
@hustcc
Copy link
Member

hustcc commented Sep 21, 2020

感觉只能这样增加一个配置了。@me-momo 有空一起帮忙看看加这样的一个配置项是否 ok?是否有其他更好的办法。

@arcsin1
Copy link
Contributor Author

arcsin1 commented Sep 21, 2020

感觉只能这样增加一个配置了。@me-momo 有空一起帮忙看看加这样的一个配置项是否 ok?是否有其他更好的办法。

其实我还有个想法的,就是通用label给measure; 用另外一种做一个包含 ,原来有个bulletStyle那个api
,我想到是否可以扩展下,我自己用:

type BasicStyle = {
 color?: ColorAttr;
 style?: StyleAttr;
 size?: SizeAttr;
 label?: GeometryLabelCfg;
}
bulletStyle: {
   measure?: BasicStyle;
   target?: BasicStyle;
   range?: BasicStyle;
}

readonly label?: GeometryLabelCfg;

/** target 的 label 设置 */
readonly targetLabel?: GeometryLabelCfg;

/** bulletStyle 包含了 measure,target,range */
readonly bulletStyle?: {
measure?: BasicStyle;
Copy link
Member

Choose a reason for hiding this comment

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

顺便提一嘴,为什么 style 中不是直接 StyleAttr 类型,(color、size 在 style 中都是可以配置出来的)

bulletStyle: {
   measure?: StyleAttr;
   target?: StyleAttr;
   range?: StyleAttr;
}

Copy link
Member

Choose a reason for hiding this comment

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

@arcsin1 这里应该要改掉。

Copy link
Contributor Author

@arcsin1 arcsin1 Sep 21, 2020

Choose a reason for hiding this comment

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

遗留问题,这是我在公用styleAttr改版之前写的,我改下

-- 不对 StyleAttr 没有color和size属性
export type StyleAttr = ShapeStyle | ((datum: Datum) => ShapeStyle);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type BasicStyle = {
 color?: ColorAttr;
 style?: StyleAttr;
 size?: SizeAttr;
 label?: GeometryLabelCfg;
}

color、size 不仅仅是 style,不应该属于 BasicStyle

我看了export type StyleAttr = ShapeStyle | ((datum: Datum) => ShapeStyle); 没有color和size

readonly label?: GeometryLabelCfg;

/** target 的 label 设置 */
readonly targetLabel?: GeometryLabelCfg;
Copy link
Member

Choose a reason for hiding this comment

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

我早上在车上想到的是:

label: {
     measure?: GeometryLabelCfg;
     target?: GeomeryLabelCfg;
}

子弹图与之不同的是,measure、target、range 都是必须的;label 上支持 measure、target 的配置,style 上支持 measure、target、range 的配置

Copy link
Member

Choose a reason for hiding this comment

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

@arcsin1 恩恩这样相对来说统一一些。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这种也可以,但是图表component自带有个label呢

Copy link
Member

Choose a reason for hiding this comment

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

什么意思?子弹图除了这两个 label,还有其他的么?

@hustcc
Copy link
Member

hustcc commented Sep 21, 2020

type BasicStyle = {
color?: ColorAttr;
style?: StyleAttr;
size?: SizeAttr;
label?: GeometryLabelCfg;
}
bulletStyle: {
measure?: BasicStyle;
target?: BasicStyle;
range

确实 label 也可以作为 geometry 的一个基础属性。

@visiky
Copy link
Member

visiky commented Sep 21, 2020

type BasicStyle = {
 color?: ColorAttr;
 style?: StyleAttr;
 size?: SizeAttr;
 label?: GeometryLabelCfg;
}

color、size 不仅仅是 style,不应该属于 BasicStyle

@arcsin1
Copy link
Contributor Author

arcsin1 commented Sep 21, 2020

@hustcc @me-momo
有个文件可以关注下
image

@visiky
Copy link
Member

visiky commented Sep 21, 2020

@hustcc @me-momo
有个文件可以关注下
image

不建议多包一层 BasicStyle

@hustcc
Copy link
Member

hustcc commented Sep 23, 2020

目前有几种配置的方式,有确定下来嘛?确定一个之后执行掉吧 ^_^!

@arcsin1
Copy link
Contributor Author

arcsin1 commented Sep 23, 2020

目前有几种配置的方式,有确定下来嘛?确定一个之后执行掉吧 ^_^!

已经确定了这种实现 @me-momo 知道这个

src/plots/bullet/types.ts Outdated Show resolved Hide resolved
src/plots/bullet/types.ts Outdated Show resolved Hide resolved
src/plots/bullet/types.ts Outdated Show resolved Hide resolved
src/plots/bullet/types.ts Outdated Show resolved Hide resolved
@hustcc hustcc merged commit 0d5e4dc into antvis:master Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PR: merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants