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.

Sign up for GitHub

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

Jump to bottom

Add preset color Tag #4571

Merged
merged 4 commits into from
Jan 13, 2017
Merged

Add preset color Tag #4571

merged 4 commits into from
Jan 13, 2017

Conversation

afc163
Copy link
Member

@afc163 afc163 commented Jan 11, 2017

close #4528

@mention-bot
Copy link

@afc163, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjycui, @yesmeck and @jljsj33 to be potential reviewers.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.274% when pulling ba76290c4318192a59faca224a8f4a7be199db49 on feat-preset-color-tag into 47fcd76 on feature-2.7.

@afc163 afc163 force-pushed the feat-preset-color-tag branch from ba76290 to 5825a5e Compare January 11, 2017 13:28
@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.274% when pulling 5825a5e on feat-preset-color-tag into 47fcd76 on feature-2.7.

@benjycui
Copy link
Contributor

对于 presetColor 和 color 这种互斥的属性，一直没想到好的处理方法。。

@afc163
Copy link
Member Author

afc163 commented Jan 12, 2017 •
edited
Loading

保证 color 的优先级会覆盖 presetColor，应该就够了。

@yesmeck
Copy link
Member

yesmeck commented Jan 12, 2017 •
edited
Loading

这个不能设计成 color 既可以是 hex 值，也可以是 color 名称吗？

@afc163
Copy link
Member Author

afc163 commented Jan 12, 2017 •
edited
Loading

原来的逻辑就是这样的。

不优雅的地方在于 color="blue" 是展示原生的 "blue" 色，还是预设的蓝色。

@afc163
Copy link
Member Author

afc163 commented Jan 12, 2017 •
edited
Loading

想了下，那几个默认色值也很丑，估计不会有人用到。就统一用回 color 吧。

@afc163
Copy link
Member Author

afc163 commented Jan 12, 2017

本次预设的色板涵盖了 1.x 里的几个预设值，相应添加的 warning 也去掉了。

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.274% when pulling 7bb9569 on feat-preset-color-tag into 47fcd76 on feature-2.7.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.274% when pulling 6bfd956 on feat-preset-color-tag into 47fcd76 on feature-2.7.

benjycui
benjycui reviewed Jan 12, 2017
View reviewed changes
components/tag/index.tsx
if (/-inverse/.test(color)) {
return true;
}
return /pink|red|yellow|orange|cyan|green|blue|purple/.test(color);
Copy link
Contributor

Choose a reason for hiding this comment

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

和上面的那个 -inverse 正则可以合并到一起？

/^(pink|red|yellow|orange|cyan|green|blue|purple)(-inverse)?$/

Copy link
Member Author

Choose a reason for hiding this comment

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

👌

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.274% when pulling a5daab8 on feat-preset-color-tag into 47fcd76 on feature-2.7.

@afc163 afc163 merged commit 1039641 into feature-2.7 Jan 13, 2017
@afc163 afc163 deleted the feat-preset-color-tag branch January 13, 2017 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjycui benjycui benjycui left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@afc163 @mention-bot @coveralls @benjycui @yesmeck