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

support custom track & handle style, for react-tiny #249

Merged
merged 1 commit into from
Apr 7, 2017

Conversation

silentcloud
Copy link
Member

No description provided.

@silentcloud silentcloud force-pushed the custom-handle-track branch from 2a51f4a to 11d765e Compare April 6, 2017 09:59
@coveralls
Copy link

Coverage Status

Coverage increased (+1.006%) to 50.8% when pulling 11d765e on custom-handle-track into 39d5e8b on master.

@@ -102,6 +102,10 @@ ReactDOM.render(
<Slider tipFormatter={null} onChange={log} />
</div>
<div style={style}>
<p>custo Slider handle and track style</p>
Copy link
Member

Choose a reason for hiding this comment

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

custom typo

Copy link
Member Author

Choose a reason for hiding this comment

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

@silentcloud silentcloud force-pushed the custom-handle-track branch from 11d765e to b9c73bb Compare April 7, 2017 02:11
@coveralls
Copy link

Coverage Status

Coverage increased (+1.006%) to 50.8% when pulling b9c73bb on custom-handle-track into 39d5e8b on master.

@benjycui
Copy link
Member

benjycui commented Apr 7, 2017

Why not just trackStyle and handleStyle, then we can remove these APIs.

  • minimumTrackTintColor
  • maximumTrackTintColor
  • handleColor
  • handleSize
  • trackSize

@silentcloud
Copy link
Member Author

silentcloud commented Apr 7, 2017

@benjycui for react-tiny, gitlab/react-tiny/issue/85, use xxx-color or xxx-size;

trackStyle can not distinguish between minimum and maximum color;

handleSize & handleColor can be replaced with handleStyle ;

but https://github.com/react-component/slider/pull/249/files#diff-9c6be5d20cab58fc76addf3a454a2dafR20;

Normally it is enough to custom style use color & size

@paranoidjk
Copy link
Member

@silentcloud
I agree rc-slider to use trackStyle and handleStyle api as @benjycui suggest.
but react-tiny can still use below api, just do a wrap.

minimumTrackTintColor
maximumTrackTintColor
handleColor
handleSize
trackSize

@paranoidjk
Copy link
Member

trackStyle may split to minimumTrackStyle and maximumTrackStyle

@silentcloud silentcloud force-pushed the custom-handle-track branch from b9c73bb to c3df6dc Compare April 7, 2017 06:11
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 49.793% when pulling c3df6dc on custom-handle-track into 39d5e8b on master.

style.borderColor = minimumTrackTintColor;
}
return <div {...restProps} className={className} style={style} />;
return <div {...restProps} className={className} style={assign({}, style, handleStyle)} />;
Copy link
Member

Choose a reason for hiding this comment

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

disabled 的背景色逻辑不需要了?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why import object-assign here when you're literally spreading objects all over?

This should be style={{...style, ...handleStyle}}

Copy link
Member

Choose a reason for hiding this comment

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

@STRML fixed in 441779c

@paranoidjk
Copy link
Member

track 有个 border-radius 而 rail 没有,底色不同的时候左侧就会露出来 @silentcloud 也顺手改下吧

https://github.com/react-component/slider/blob/master/assets/index.less#L43

https://github.com/react-component/slider/blob/master/assets/index.less#L32

@silentcloud silentcloud force-pushed the custom-handle-track branch from c3df6dc to a1d31f1 Compare April 7, 2017 06:34
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 50.0% when pulling a1d31f1 on custom-handle-track into 39d5e8b on master.

@silentcloud silentcloud force-pushed the custom-handle-track branch from a1d31f1 to 1ca4303 Compare April 7, 2017 06:39
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 50.0% when pulling 1ca4303 on custom-handle-track into 39d5e8b on master.

}
return <div className={className} style={style} />;

const trackStyle = disabled ? style : assign({}, style, minimumTrackStyle);
Copy link
Member

Choose a reason for hiding this comment

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

这样感觉有点怪,minimumTrackStyle 用处是自定义样式,除了颜色还有宽高也可能会变,这样如何:

   const baseTrackStyle = assign({}, style, minimumTrackStyle);
   const disabledTrackStyle = {
        // 原来 disable 的样式逻辑
    };
   const trackStyle = disabled ? assign(baseTrackStyle, disabledTrackStyle) : baseTrackStyle;

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 Author

Choose a reason for hiding this comment

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

@paranoidjk 我就是按照没加 minimumTintColor 的时候的逻辑搞的

Copy link
Member

Choose a reason for hiding this comment

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

恩,主要是我们的api 从 minimumTintColor 自定义颜色 扩大到了 minimumTrackStyle 自定义样式,如果用户发现 disable 的时候他自定义的样式全没了,就不太合理。

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

我直接把 disabled 去掉好了

Copy link
Member

Choose a reason for hiding this comment

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

ok,把 disabled 的样式逻辑去掉也行。

@silentcloud silentcloud force-pushed the custom-handle-track branch from 1ca4303 to db4a0f1 Compare April 7, 2017 07:00
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 49.793% when pulling db4a0f1 on custom-handle-track into 39d5e8b on master.

@paranoidjk paranoidjk merged commit cbb3b8a into master Apr 7, 2017
@paranoidjk paranoidjk deleted the custom-handle-track branch April 7, 2017 07:16
@paranoidjk
Copy link
Member

@silentcloud 7.0.0

@@ -105,8 +105,9 @@ 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 | | The color used for the track to the left of the button. |
| maximumTrackTintColor | String | | The color used for the track to the right of the button. |
| minimumTrackStyle | Object |`{}` | The style 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.

看了源码和内网 issue 才知道这个 API 是干嘛用的 😢

Copy link
Member Author

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.

对于单滑块,这个命名是没问题的,但对于多滑块,就含义不清了。。不过算了,先用着看看吧。

Copy link
Member Author

Choose a reason for hiding this comment

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

恩多滑块貌似是不适合,当时也没想到好名字,就跟着 RN 来了

@gragland
Copy link
Contributor

gragland commented Apr 7, 2017

Was the handleSize prop also added to Range or just Slider? Would love to have this functionality for Range as well.

Senior-Hayato-Suzuki added a commit to Senior-Hayato-Suzuki/react-slider that referenced this pull request Feb 26, 2020
blue-git-pro added a commit to blue-git-pro/dashboard-next.js that referenced this pull request May 14, 2021
web3gru pushed a commit to web3gru/slider that referenced this pull request May 13, 2023
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.

6 participants