add h5number type for inputItem #1419

pingan1927 · 2017-06-06T05:44:27Z

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

mention-bot · 2017-06-06T05:44:28Z

@pingan1927, thanks for your PR! By analyzing the history of the files in this pull request, we identified @chinalife, @warmhug and @silentcloud to be potential reviewers.

codecov · 2017-06-06T05:51:38Z

Codecov Report

Merging #1419 into master will increase coverage by 0.12%.
The diff coverage is 52.83%.

@@            Coverage Diff            @@
##           master   #1419      +/-   ##
=========================================
+ Coverage   61.98%   62.1%   +0.12%     
=========================================
  Files         220     222       +2     
  Lines        4201    4455     +254     
  Branches     1236    1327      +91     
=========================================
+ Hits         2604    2767     +163     
- Misses       1596    1687      +91     
  Partials        1       1

Flag	Coverage Δ
#rn	`63.74% <ø> (+0.38%)`	⬆️
#web	`60.53% <52.83%> (-0.05%)`	⬇️

Impacted Files	Coverage Δ
components/input-item/index.web.tsx	`48.27% <0%> (+2.66%)`	⬆️
components/input-item/CustomInput.web.tsx	`33.33% <33.33%> (ø)`
components/input-item/CustomKeyboard.web.tsx	`81.81% <83.33%> (ø)`
components/modal/operation.web.tsx	`7.69% <0%> (-2.31%)`	⬇️
components/button/index.web.tsx	`94.73% <0%> (-2.04%)`	⬇️
components/modal/alert.web.tsx	`6.45% <0%> (-1.55%)`	⬇️
components/modal/prompt.web.tsx	`2.5% <0%> (-0.49%)`	⬇️
components/pagination/index.web.tsx	`77.77% <0%> (+0.27%)`	⬆️
... and 7 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 252383a...794bf2c. Read the comment docs.

paranoidjk · 2017-06-06T06:01:25Z

components/input-item/H5NumberKeyBoard.tsx

+      <table>
+        <tbody>
+          <tr>
+            <KeyboardItem onClick={this.onKeyBoardClick}>1</KeyboardItem>


这么多重复代码，优化一下？可以用function来创建重复的元素

同意，这里可以优化一下

就这样吧，有的要设置rowspan 有的要设置特定className 有个一行4个元素有的一行3个元素，function创建还要加一堆判断。

这块改了

paranoidjk · 2017-06-06T06:02:36Z

我们的 Icon 不都是用 svg 么，这里为啥是 png？

silentcloud · 2017-06-06T06:02:48Z

components/input-item/H5NumberKeyBoard.tsx

+      <table>
+        <tbody>
+          <tr>
+            <KeyboardItem onClick={this.onKeyBoardClick}>1</KeyboardItem>


同意，这里可以优化一下

silentcloud · 2017-06-06T06:04:28Z

components/input-item/index.web.tsx

@@ -4,6 +4,7 @@ import classNames from 'classnames';
 import omit from 'omit.js';
 import InputItemProps from './PropsType';
 import Input from './Input.web';
+import H5NumberInput from './H5NumberInput.web';


这里感觉不内置会好一点，搞成一个子组件，在用户需要的时候自己引入

这里还是我们没有按需加载的问题，bundle体积会增大

恩就是这个问题

这个我也考虑过，子组件构建时不会搞进来？比如Checkbox 和 Checkbox.CheckboxItem，如果没有用到CheckboxItem，相关代码不会引入？

不是那样的，是类似 import h5Input from 'antd-mobile/lib/input/h5input'

silentcloud · 2017-06-06T06:11:28Z

这个算是一个锦上添花功能，因为不是必须，只有用到这个场景了才需要，最好是做成 InputItem 的一个子组件来搞，不内置在 InputItem 里 @pingan1927

paranoidjk · 2017-06-06T06:16:38Z

components/input-item/H5NumberInput.web.tsx

+      if (value === '') {
+        onChange({ target: { value: '' } });
+      } else {
+        onChange({ target: { value: value.substring(0, value.length - 1) } });


这里不需要 if else的，else里的逻辑就够了

阔以，再简化下。

paranoidjk · 2017-06-06T06:20:59Z

components/input-item/index.web.tsx

+              onBlur={this.onInputBlur}
+              disabled={disabled}
+              value={fixControlledValue(value)}
+              {...(this.props.focused !== undefined ? { focused: this.props.focused } : {})}


这里意图是要区分 !(focused in this.props) 和 this.props.focused === undefined 这两种情况？

paranoidjk · 2017-06-06T06:26:29Z

components/input-item/H5NumberInput.web.tsx

+    document.addEventListener('click', (ev) => {
+      if (this.refs['input-container'] !== ev.target &&
+        this.refs['input-container'] !== (ev.target as any).closest(`.${prefixCls}-control`) &&
+        (ev.target as any).closest(`.${keyboardPrefixCls}-wrapper`) === null) {


我理解这里意图是要区分点击是否发生在虚拟键盘区域，

在虚拟键盘的最外层 div 上 stopPropagation，再直接监听 document 上的 click，是否就可以认为此时的 click 是在键盘区域外了？

有两处，如果点击了输入框以及对应的虚拟键盘外的地方就触发onBlur。

我理解 点击了输入框以及对应的虚拟键盘外的地方 就是一种 点击了当前虚拟键盘之外的区域。

所以建议尝试一下 stopPropagation 的方案

paranoidjk · 2017-06-06T06:28:54Z

components/input-item/H5NumberInput.web.tsx

+import H5NumberKeyBoard from './H5NumberKeyBoard';
+
+if ((window as any).Element && !Element.prototype.closest) {
+  Element.prototype.closest =


我的观点，同下面所讨论的 stopPropagation 方案，需要遍历 dom 的，甚至要 polyfill dom api的，应该是最后的选择，优先看其他方案。

这个有更好的方案就放出来，需求很明确的，如果有更好的写法最好了。

这段能不能移到 componentDidMount 里面去。
或者不碰 prototype，就写个函数 function closest(el, s)，因为下面只有一处用到这个 closest 功能。

@cncolder 这个我再重点考虑下，目前的PR放出来就是让讨论有的放矢。

pingan1927 · 2017-06-06T06:50:46Z

我们的 Icon 不都是用 svg 么，这里为啥是 png？

这两个native的都是png，td里套svg不知道会不会带来麻烦（包div后垂直居中很难搞），我在斟酌下。不引入其他组件的前提下可以搞成svg。

paranoidjk · 2017-06-06T06:53:18Z

components/input-item/H5NumberKeyBoard.tsx

+      [`${prefixCls}-wrapper-hide`]: hide,
+    });
+    return (<div className={wrapperCls}>
+      <table>


用 table 是为了布局考虑？ Flex 应该很容易实现宫格布局吧，类似我们的 Grid

用table标签是最佳方案，rowspan等参数就是为这个场景设置的。

好吧.. 用 rowspan 来做这个 button 的布局的确挺巧妙的。

paranoidjk · 2017-06-07T11:11:27Z

components/input-item/demo/money.md

+            placeholder="money keyboard"
+            clear
+            maxLength={10}
+            autoFocus


这个demo就不要 autoFocus了, 其他demo里也有个autoFocus, 真机上会同时弹出两个键盘

是该去掉

paranoidjk · 2017-06-07T11:16:16Z

components/input-item/CustomInput.web.tsx

+      >
+        {value}
+      </div>
+      <CustomKeyboard


这里会导致一个页面可能有多个键盘实例，dom 冗余不说，事件也就会有多重绑定了。

键盘的 dom 应该是仅仅只有一次，一个页面也只能同时准许弹出一个键盘实例

这个也是一个纠结点，类似的共享模块的实现有没有，有就用，没有就初始化一个，虚拟键盘实例绑定在window对象？但这也破坏了组件的内聚性了

paranoidjk · 2017-06-07T11:45:00Z

components/input-item/CustomKeyboard.web.tsx

+    confirmDisabled: false,
+  };
+  onKeyboardClick = (e, value) => {
+    e.stopPropagation();


这里改成 e.nativeEvent.stopImmediatePropagation();

同时在 onFakeInputClick 里也加上 e.nativeEvent.stopImmediatePropagation(); document on click 里面的那些 closet 读取 dom 的逻辑都可以去掉了

这个办法牛逼！

贴一下原理解释在代码注释里吧。https://stackoverflow.com/questions/24415631/reactjs-syntheticevent-stoppropagation-only-works-with-react-events/24421834#24421834

paranoidjk · 2017-06-07T11:52:50Z

components/input-item/CustomInput.web.tsx

+    if (autoFocus || focused) {
+      this.onInputFocus(value);
+    }
+    document.addEventListener('click', (ev) => {


componentWillUnmount 里面删除事件，避免内存泄露。

cncolder · 2017-06-07T12:01:07Z

components/input-item/CustomInput.web.tsx

+import React from 'react';
+import CustomKeyboard from './CustomKeyboard.web';
+
+if ((window as any).Element && !Element.prototype.closest) {


这个 window 是在 require 阶段引用的，node 环境没法 require 这个文件。

@cncolder 看我上面的评论，阻止冒泡的方法不需要再 polyfill closest，这段代码可以删除

paranoidjk · 2017-06-09T07:17:55Z

真机上体验下来还是有些问题的：

删除的效果不太靠谱，又是删除无效
长按删除可以支持加速的删除内容
很难移动光标位置（普通的InputItem也有这个问题）

pingan1927 · 2017-06-09T09:17:52Z

很难移动光标位置（普通的InputItem也有这个问题）

这个放弃，不支持。现在JSAPI里的实现也有bug。

长按删除可以支持加速的删除内容

金额键盘的核心功能是可以支持小数点输入，其他的都是次要需求。如果后续长按快速删除的需求特别强烈，我们再继续优化。

删除的效果不太靠谱，又是删除无效。

这个你是极端测试吧，H5的体验就是会差一点。

再次强调一点，核心需求是提供带小数点的数字键盘，不需要用户自己切换键盘，且不能输入其他字符。

* add h5number type for inputItem * fix bug for h5number input * optimize * fix name error * optimize code * optimize code

pingan1927 added 2 commits June 6, 2017 13:42

add h5number type for inputItem

2cf86e0

fix bug for h5number input

f259077

pingan1927 requested a review from silentcloud June 6, 2017 05:44

silentcloud assigned silentcloud and paranoidjk Jun 6, 2017

paranoidjk reviewed Jun 6, 2017

View reviewed changes

silentcloud reviewed Jun 6, 2017

View reviewed changes

paranoidjk reviewed Jun 6, 2017

View reviewed changes

paranoidjk suggested changes Jun 6, 2017

View reviewed changes

paranoidjk reviewed Jun 6, 2017

View reviewed changes

pingan1927 added 3 commits June 6, 2017 16:41

optimize

a3576c9

fix name error

1f8585a

optimize code

33ce784

paranoidjk suggested changes Jun 7, 2017

View reviewed changes

cncolder reviewed Jun 7, 2017

View reviewed changes

optimize code

794bf2c

paranoidjk approved these changes Jun 9, 2017

View reviewed changes

paranoidjk merged commit cd78274 into master Jun 9, 2017

paranoidjk deleted the customNumberInuput branch June 9, 2017 09:55

add h5number type for inputItem #1419

add h5number type for inputItem #1419

Conversation

pingan1927 commented Jun 6, 2017 • edited by paranoidjk Loading

mention-bot commented Jun 6, 2017

codecov bot commented Jun 6, 2017 • edited Loading

Codecov Report

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

paranoidjk commented Jun 6, 2017

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

silentcloud commented Jun 6, 2017

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

cncolder Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pingan1927 commented Jun 6, 2017

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

paranoidjk commented Jun 9, 2017

pingan1927 commented Jun 9, 2017 • edited Loading

pingan1927 commented Jun 6, 2017 •

edited by paranoidjk

Loading

codecov bot commented Jun 6, 2017 •

edited

Loading

cncolder Jun 6, 2017 •

edited

Loading

pingan1927 commented Jun 9, 2017 •

edited

Loading