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

add attribute compact for InputGroup #4309

Merged
merged 2 commits into from
Jan 1, 2017
Merged

Conversation

ddcat1115
Copy link
Contributor

close #4186

@mention-bot
Copy link

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

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.523% when pulling 563f30b99123f321e7de17cc21de3e3f8894a9cb on ddcat1115:fix-4186 into 0e92fa3 on ant-design:master.

@benjycui
Copy link
Contributor

CI failed.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.523% when pulling b537d41581e4b283e2f5e024e46b072a6bdb525e on ddcat1115:fix-4186 into 8382023 on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.523% when pulling b537d41581e4b283e2f5e024e46b072a6bdb525e on ddcat1115:fix-4186 into 8382023 on ant-design:master.

@benjycui
Copy link
Contributor

其实我一直觉得 Input.Group 里面用 Col 怪怪的,虽然这样灵活性很好。

@ddcat1115
Copy link
Contributor Author

是。。为了跟之前保持一致。。。

@benjycui
Copy link
Contributor

benjycui commented Dec 23, 2016

这样如何?

Input.Group {
  addonBefore?: React.ReactNode;
  addonAfter?: React.ReactNode;
}

事实上,现在 Input[addonBefore|addonAfter] 也是用了 Input.Group 的样式实现的: https://github.com/ant-design/ant-design/blob/master/components/input/Input.tsx#L153-L178

这样实现,你应该也不需要再添加样式了。

@benjycui
Copy link
Contributor

然后这个属于新 feature,PR 到 feature-2.6 ?

@ddcat1115
Copy link
Contributor Author

感觉这个跟addon还是不一样,InputGroup下的item应该是平级的,且不限个数,addon应该是有附庸关系的,且只能前后各加一个,并且目前addon item都默认灰色

其实感觉addon应该轻量化一点,就加加固定文案icon什么的,拼接组件还是直接用InputGroup,只是不一定要用Col来控制宽度,其实直接写在item style里也没什么关系

@benjycui
Copy link
Contributor

主要是现在这样,感觉跟没封装似的。。

然后需求不就是 addon 么?

@ddcat1115
Copy link
Contributor Author

提需求的时候我也觉得用addon好了,后来感觉不一样,title我应该改一下。。。。。目前这样对样式还是有一些封装,不然自己写起来还是有点烦

@benjycui benjycui requested a review from afc163 December 26, 2016 04:26
& > div:first-child .@{inputClass},
& > div:first-child .@{ant-prefix}-select-selection {
border-top-left-radius: @border-radius-base!important;
border-bottom-left-radius: @border-radius-base!important;
Copy link
Member

Choose a reason for hiding this comment

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

尽量不要用 !important 吧。

.@{ant-prefix}-select-selection {
border-radius: 0;
border-right-width: 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

其他的组件例如 DatePicker,Cascader,InputNumber 这些是否也要支持?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DatePicker和Cascader是基于ant-input的,这种已经可以支持,InputNumber要加下

@@ -27,6 +28,49 @@ ReactDOM.render(
<Input defaultValue="26888888" />
</Col>
</InputGroup>
<br />
<InputGroup compact>
<Col span="4">
Copy link
Member

Choose a reason for hiding this comment

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

例子里还是不要推荐用 Col 吧,直接这样如何?感觉效果是一样的,结构更清晰易懂。

<InputGroup compact>
  <Input style={{ width: '33.3% '}} />
  <Input style={{ width: '66.6% '}} />
</InputGroup>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,直接用更清晰,那之前的InputGroup的例子也需要改下,只是代码里Col-相关的样式还是不能去掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

之前的例子改不了了,只能是,因为之前 .ant-input-group 针对没有 Col- 的也有设置样式(因为addon),非 compact 模式的如果去掉 Col 就有冲突。。。。反正比较烦人,只能新加入的compact 模式不用 Col

.@{inputClass},
.@{ant-prefix}-select-selection {
border-radius: 0;
border-right-width: 0;
Copy link
Member

Choose a reason for hiding this comment

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

要考虑 3 个和 3 个以上拼合的场景。


&&-compact {

& > [class*="col-"] {
Copy link
Member

Choose a reason for hiding this comment

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

InputGroup 和 Col 有潜规则上的耦合,这个不太好。

Copy link
Contributor 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.

原来的逻辑略蛋疼,应该用 gutter 实现。

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 71.903% when pulling b07513a2bdd395bb4198c8b37620eb9ccb045aca on ddcat1115:fix-4186 into 8382023 on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.903% when pulling eaa0c08d78bafe8e3e323884e503f0a999989b3e on ddcat1115:fix-4186 into c285f5f on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.903% when pulling 16da138107fd92356cb70f899e65268a23da3052 on ddcat1115:fix-4186 into 7f7d940 on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.903% when pulling a42c1b9850bdb49d55eb0a0b39f582d08a99392a on ddcat1115:fix-4186 into 7f7d940 on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.903% when pulling 35f43e1 on ddcat1115:fix-4186 into d811ebe on ant-design:master.

@ddcat1115 ddcat1115 merged commit c5b1542 into ant-design:master Jan 1, 2017
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.

Select 支持 addon
5 participants