fix #1290 switch support onClick #1293

pingan1927 · 2017-05-10T04:00:41Z

First of all, thanks for your contribution! :-)

Please makes sure these boxes are checked before submitting your PR, thank you!

Make sure you follow antd's code convention.
Run npm run lint and fix those errors before submitting in order to keep consistent code style.
Rebase before creating a PR to keep commit history clear.
Add some descriptions and refer relative issues for you PR.

This change is

codecov · 2017-05-10T04:06:32Z

Codecov Report

Merging #1293 into master will decrease coverage by 0.07%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master    #1293      +/-   ##
==========================================
- Coverage   61.82%   61.75%   -0.08%     
==========================================
  Files         219      219              
  Lines        4155     4165      +10     
  Branches     1222     1225       +3     
==========================================
+ Hits         2569     2572       +3     
- Misses       1585     1592       +7     
  Partials        1        1

Flag	Coverage Δ
#rn	`63.12% <ø> (ø)`	⬆️
#web	`60.35% <77.77%> (-0.15%)`	⬇️

Impacted Files	Coverage Δ
components/switch/index.web.tsx	`62.06% <77.77%> (-16.88%)`	⬇️

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 d369ecb...e38baa8. Read the comment docs.

paranoidjk · 2017-05-10T05:04:08Z

components/switch/index.web.tsx

-        <div className="checkbox" />
+        <div
+          className={fackInputCls}
+          onClick={this.onChange}


没测试？这里写错了吧

我再改改

肯定写错了啊... 应该是 onClick={this.onClick} 吧

afc163 · 2017-05-10T06:15:19Z

components/switch/index.web.tsx

+  onClick = () => {
+    const { checked } = this.props;
+    if (this.props.onClick) {
+      this.props.onClick(checked);


感觉 onClick 应该直接传 event。

afc163 · 2017-05-10T06:15:50Z

components/switch/style/index.less

@@ -52,6 +52,9 @@
      transition: all 200ms;
      box-shadow: 4px 4px 8px @color-shadow;
    }
+    &.checkbox-disabled {
+      z-index: 3;


修改 z-index 感觉很奇怪。

对应的写法

input标签的z-index是2，不改z-index 后一个元素不能被点击到

input 的 z-index 也应该去掉。

那改动就大了去了，大的写法问题可以起个issue做一次重构

afc163 · 2017-05-10T06:16:01Z

components/switch/PropsType.tsx

@@ -8,6 +8,7 @@ interface SwitchProps {
  className?: string;
  name?: string;
  platform?: string;
+  onClick?: Function;


应该补充参数。

onClick的触发是在onChange之前，取的参数是change前的值，把参数去掉了，如果要加参数，就业务自己取比较好。

参数先都不要了，onClick先于onChange触发。参数自取

@pingan1927 没任何参数不行。至少把 event 对象给出来，不然看起来就是纯粹为了你眼前的需求去加这么一个 onClick，这不太好。

现在传入e.target.checked 或 this.props.checked

div 应该没有 e.target.checked 吧，你这个 api 的定位还是不太清晰，补充一下 api 文档，像其他api一样描述一下出入参，作用

如果disabled，取的是switch组件传入的默认的checked。API已更新。

API 的定位很清晰啊，就是Switch组件本身要支持onClick，具体产生什么行为业务自己负责。现在不支持onClick，其实是bug

afc163 · 2017-05-10T06:21:33Z

components/switch/index.web.tsx

        />
        <div
          className={fackInputCls}
-          onClick={this.onChange}
+          {...(disabled ? { onClick: this.onClick } : {})}


感觉直接 <label onClick={this.props.onClick}> 即可，和 disabled 关系不大。

非disabled的时候，onClick事件放到input上，只有diabled的时候才放到用于渲染的div

label加上onClick我试试看

<label onClick={this.props.onClick}> 会受到disabled影响，不可行

afc163 · 2017-05-10T06:22:22Z

components/switch/index.web.tsx

@@ -49,10 +48,11 @@ export default class Switch extends React.Component<SwitchProps, any> {
          {...(disabled ? { disabled: 'disabled' } : {})}


这个之前的代码也看的怪怪的，disabled={disabled} 不就行了。

这个晚点review下是否会有影响，这次先不改了。

问题不要一直往后堆积嘛，发现一个就及时修复。
这个可以改掉 https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/disabled

我知道你的意思，disabled的值可以传boolean，改了

* fix ant-design#1290 * update * fix bug * fix * update API doc * update switch PropsType

fix #1290

c1ac6ef

pingan1927 changed the title ~~fix #1290~~ fix #1290 switch support onClick May 10, 2017

paranoidjk suggested changes May 10, 2017

View reviewed changes

update

040cd9c

afc163 reviewed May 10, 2017

View reviewed changes

fix bug

b638391

afc163 reviewed May 10, 2017

View reviewed changes

pingan1927 added 3 commits May 10, 2017 16:23

fix

7f2795c

update API doc

76bea2a

update switch PropsType

e38baa8

pingan1927 merged commit b3a75ff into master May 12, 2017

paranoidjk deleted the enhance-switch branch May 15, 2017 07:59

lixiaoyang1992 pushed a commit to lixiaoyang1992/ant-design-mobile that referenced this pull request Apr 26, 2018

fix ant-design#1290 switch support onClick (ant-design#1293)

2cca34b

* fix ant-design#1290 * update * fix bug * fix * update API doc * update switch PropsType

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #1290 switch support onClick #1293

fix #1290 switch support onClick #1293

pingan1927 commented May 10, 2017 •

edited

Loading

codecov bot commented May 10, 2017 •

edited

Loading

paranoidjk May 10, 2017

pingan1927 May 10, 2017 •

edited

Loading

paranoidjk May 10, 2017

pingan1927 May 10, 2017

afc163 May 10, 2017

afc163 May 10, 2017

pingan1927 May 10, 2017

pingan1927 May 10, 2017

afc163 May 10, 2017

pingan1927 May 10, 2017

afc163 May 10, 2017

pingan1927 May 10, 2017

pingan1927 May 10, 2017

paranoidjk May 10, 2017

pingan1927 May 10, 2017

paranoidjk May 12, 2017

pingan1927 May 12, 2017

pingan1927 May 12, 2017

afc163 May 10, 2017

pingan1927 May 10, 2017

pingan1927 May 10, 2017

pingan1927 May 10, 2017 •

edited

Loading

afc163 May 10, 2017

pingan1927 May 10, 2017

paranoidjk May 10, 2017

pingan1927 May 10, 2017

		@@ -49,10 +48,11 @@ export default class Switch extends React.Component<SwitchProps, any> {
		{...(disabled ? { disabled: 'disabled' } : {})}

fix #1290 switch support onClick #1293

fix #1290 switch support onClick #1293

Conversation

pingan1927 commented May 10, 2017 • edited Loading

codecov bot commented May 10, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

pingan1927 May 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingan1927 May 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingan1927 commented May 10, 2017 •

edited

Loading

codecov bot commented May 10, 2017 •

edited

Loading

pingan1927 May 10, 2017 •

edited

Loading

pingan1927 May 10, 2017 •

edited

Loading