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(Icon) Icon with theme and colorful icon #11971

Merged
merged 46 commits into from Sep 2, 2018

Conversation

HeskeyBaozi
Copy link
Contributor

@HeskeyBaozi HeskeyBaozi commented Aug 30, 2018

Features

  • New prop theme to <Icon />. It can specific the theme of icons.

import { Icon } from 'antd';

ReactDOM.render(
  <div className="icons-list">
    <Icon type="home" />
    <Icon type="setting" theme="filled" />
    <Icon type="smile" theme="outlined" />
    <Icon type="sync" spin />
    <Icon type="loading" />
  </div>,
  mountNode
);
<Icon type="setting" theme="filled" />
<Icon type="smile" theme="outlined" />
<Icon type="dollar" theme="twoTone" />
//        ⬇️
// is equivalent to
//        ⬇️
<Icon type="setting-fill" />
<Icon type="smile-o" />
<Icon type="dollar-twotone" />
  • Two-tone icons

import { Icon } from 'antd';

Icon.setTwoToneColors({
  primaryColor: '#1890ff',
});

ReactDOM.render(
  <div className="icons-list">
    <Icon type="dollar" theme="twoTone" />
    <Icon type="euro" theme="twoTone" />
    <Icon type="check-circle" theme="twoTone" primaryColor="#f5222d" />
  </div>,
  mountNode
);

Bug fixes

c1f63a1
The components notification, message, progress: status error use close(-circle) icon instead of cross(-circle) icon.

WIP

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Aug 30, 2018

Deploy preview for ant-design ready!

Built with commit 82746c0

https://deploy-preview-11971--ant-design.netlify.com

@yesmeck
Copy link
Member

yesmeck commented Aug 30, 2018

themefilledoutlinedtwo-tone?

@codecov
Copy link

codecov bot commented Aug 31, 2018

Codecov Report

Merging #11971 into feature-3.9.0 will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                @@
##           feature-3.9.0   #11971      +/-   ##
=================================================
+ Coverage          92.34%   92.36%   +0.02%     
=================================================
  Files                207      208       +1     
  Lines               5405     5446      +41     
  Branches            1517     1526       +9     
=================================================
+ Hits                4991     5030      +39     
- Misses               410      412       +2     
  Partials               4        4
Impacted Files Coverage Δ
components/pagination/Pagination.tsx 100% <ø> (ø) ⬆️
components/tree-select/index.tsx 83.78% <ø> (ø) ⬆️
components/notification/index.tsx 95.65% <ø> (ø) ⬆️
components/date-picker/WeekPicker.tsx 80.39% <ø> (ø) ⬆️
components/switch/index.tsx 100% <ø> (ø) ⬆️
components/date-picker/RangePicker.tsx 95.17% <ø> (ø) ⬆️
components/collapse/Collapse.tsx 100% <ø> (ø) ⬆️
components/date-picker/createPicker.tsx 97.01% <ø> (ø) ⬆️
components/rate/index.tsx 100% <ø> (ø) ⬆️
components/steps/index.tsx 100% <100%> (ø) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 175ae52...82746c0. Read the comment docs.

@yesmeck
Copy link
Member

yesmeck commented Aug 31, 2018

说错了,应该是 twoTone

};
}
render() {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

应该是两个空格的缩进。

Copy link
Member

Choose a reason for hiding this comment

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

不对,这是 ts 生成的文件,那不应该提交上来。

@@ -0,0 +1,23 @@
/* eslint-disable */
Copy link
Member

Choose a reason for hiding this comment

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

这个很不好哦。

return null;
}
}
IconDisplay.displayName = 'IconDisplay';
Copy link
Member

Choose a reason for hiding this comment

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

没必要写 displayName。

@@ -20,7 +20,7 @@ const CopyableIcon: React.SFC<CopyableIconProps> = ({
onCopy={() => onCopied(type)}
>
<li className={justCopied === type ? 'copied' : ''}>
<Icon type={type} theme={theme} />
<Icon type={type} theme={theme} primaryColor="#333" secondaryColor="#e6e6e6" />
Copy link
Member

Choose a reason for hiding this comment

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

secondaryColor 应该是直接用 primaryColor 算出来的吧?

Copy link
Member

@afc163 afc163 Sep 1, 2018

Choose a reason for hiding this comment

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

primaryColor 应该是只给 twoTone 用的,不如就叫 twoToneColor,和 setTwoToneColors 保持一致。

setTwoToneColors 也改为 setTwoToneColor('xxx') 好了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

默认的主色是 #333, 副色是 #e6e6e6
用ant-design-palettes无法生成正确的副色

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这样

const colors = {
  primary: '#333',
  secondary: '#E6E6E6', // optional
};

<Icon type={type} theme={theme} twoToneColors={colors} />

如何

Copy link
Contributor Author

Choose a reason for hiding this comment

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

保留副色的接口是因为 ant-design-palettesgenerate 函数生成的副色有时无法使用,如
generate('#333') 生成的十个颜色都太深了

Copy link
Member

@afc163 afc163 Sep 1, 2018

Choose a reason for hiding this comment

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

#333 到 #e6e6e6 是怎么算出来的呢,不要用 ant-design-palettes,单独写一个转换。

Copy link
Member

Choose a reason for hiding this comment

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

不要让用户去设他们能力范围之外的属性。

Copy link
Member

Choose a reason for hiding this comment

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

默认主色应该是 antd 的主色。

Copy link
Member

Choose a reason for hiding this comment

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

TwoTone 的 默认效果应该类似 https://ego-icons.com/

@afc163
Copy link
Member

afc163 commented Sep 1, 2018

svgClassName 和 svgStyle 貌似没啥必要,建议去掉。

@afc163
Copy link
Member

afc163 commented Sep 1, 2018

文档补充一段提示吧,3.9.0 之后新增了哪些 API,这样开发者如果用老版本不容易搞错。

@HeskeyBaozi
Copy link
Contributor Author

HeskeyBaozi commented Sep 1, 2018

svgClassName 可以去掉,替代方案是用户使用元素选择器 .anticon svg 设置样式
svgStyle 用来实现图标的旋转、翻转等效果。这个高优先级的内联样式我觉得接口保留会好一点。
了解需求

@@ -89,8 +89,9 @@ ReactDOM.render(
| className | 计算后的 `svg` 类名 | string | - |
| style | 计算后的 `svg` 元素样式 | CSSProperties | - |

#### 使用 iconfont.cn 的自定义图标
Copy link
Member

Choose a reason for hiding this comment

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

用三个 ###

上面的 ### Icon 可以去掉,没啥用。

@afc163
Copy link
Member

afc163 commented Sep 2, 2018

冲突了又,解决了先合并吧。

@HeskeyBaozi HeskeyBaozi force-pushed the feature/icon-with-theme-and-colorful-icon branch from 0027e98 to 4ea3aae Compare September 2, 2018 10:26
@@ -320,6 +320,7 @@ class RangePicker extends React.Component<any, RangePickerState> {
type="close-circle"
className={`${prefixCls}-picker-clear`}
onClick={this.clearSelection}
theme={'filled'}
Copy link
Member

Choose a reason for hiding this comment

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

theme="filled"

@HeskeyBaozi HeskeyBaozi force-pushed the feature/icon-with-theme-and-colorful-icon branch from f44b078 to e784ac3 Compare September 2, 2018 11:02
@HeskeyBaozi HeskeyBaozi changed the title WIP: feat(Icon) Icon with theme and colorful icon feat(Icon) Icon with theme and colorful icon Sep 2, 2018
@afc163 afc163 merged commit 52f1625 into feature-3.9.0 Sep 2, 2018
@afc163 afc163 deleted the feature/icon-with-theme-and-colorful-icon branch September 2, 2018 12:59
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

4 participants