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

feature: support minimumTrackTintColor & maximumTrackTintColor #243

Merged
merged 2 commits into from Mar 22, 2017

Conversation

silentcloud
Copy link
Member

@coveralls
Copy link

Coverage Status

Coverage increased (+3.8%) to 49.794% when pulling 625f9a4 on support-color into 0017ed4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.8%) to 49.794% when pulling 5b2a9b0 on support-color into 0017ed4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.8%) to 49.794% when pulling 5b2a9b0 on support-color into 0017ed4 on master.

README.md Outdated
@@ -105,6 +105,8 @@ The following APIs are shared by Slider and Range.
| onBeforeChange | Function | NOOP | `onBeforeChange` will be triggered when `ontouchstart` or `onmousedown` is triggered. |
| onChange | Function | NOOP | `onChange` will be triggered while the value of Slider changing. |
| onAfterChange | Function | NOOP | `onAfterChange` will be triggered when `ontouchend` or `onmouseup` is triggered. |
| minimumTrackTintColor | String | #e9e9e9 | The color used for the track to the left of the button. |
Copy link
Member

Choose a reason for hiding this comment

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

@silentcloud 把这个默认值体现在 defaultProps 里面吧,放到css里面写死,后面维护性不是很好

Copy link
Member Author

Choose a reason for hiding this comment

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

@paranoidjk 不放在 defaultProps 里的原因是:用户可以直接通过 theme 来定义,而不用传这个参数,react-tiny 里没办法定义 theme,所以需要这两个 prop

Copy link
Member

Choose a reason for hiding this comment

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

没get到你的意思,在react-component这层它是没有theme的概念的,直接用props当配置项

Copy link
Member

Choose a reason for hiding this comment

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

在antd-mobile那层同时保留theme和props就行,props定义可以覆盖theme

Copy link
Member Author

Choose a reason for hiding this comment

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

@paranoidjk 意思就是说,要 theme 的优先级高于 prop,用户优先以全局 theme 来定义 color

Copy link
Member Author

Choose a reason for hiding this comment

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

因为代码里用的 style,传了 defaultProps 之后,style 的 color 优先级就高了,theme 就没作用了

Copy link
Member

Choose a reason for hiding this comment

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

因为代码里用的 style,传了 defaultProps 之后,style 的 color 优先级就高了,theme 就没作用了

发个代码链接我看下?

Copy link
Member

Choose a reason for hiding this comment

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

还是没理清楚你这个需求的背景,按我的理解,antd-mobile是基于rc-component, react-tiny不是在antd-mobile之上的吗? 为啥在rc-sider这层还要纠结于theme的处理,rc-slider就只有props才对

@paranoidjk paranoidjk merged commit 3c7ce8f into master Mar 22, 2017
@paranoidjk paranoidjk deleted the support-color branch March 22, 2017 03:56
@paranoidjk
Copy link
Member

@silentcloud 6.3.0

@silentcloud
Copy link
Member Author

ok

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

3 participants