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

firstActiveValue #201

Merged
merged 11 commits into from
Jul 27, 2017
Merged

firstActiveValue #201

merged 11 commits into from
Jul 27, 2017

Conversation

L-x-C
Copy link
Contributor

@L-x-C L-x-C commented Jun 14, 2017

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.585% when pulling 1f33234 on L-x-C:firstActiveValue into 57f1219 on react-component:master.

@L-x-C
Copy link
Contributor Author

L-x-C commented Jun 28, 2017

@yesmeck 求个反馈

@yesmeck
Copy link
Member

yesmeck commented Jun 28, 2017

不好意思,晚上看下。

@yesmeck
Copy link
Member

yesmeck commented Jun 28, 2017

试了下好像并没有效果,你能写个例子吗?

@L-x-C
Copy link
Contributor Author

L-x-C commented Jun 28, 2017

里面放了个列子的,跑起来后,http://localhost:8003/examples/single.html

image

image

image

把那个默认值9叉掉,在没有默认选项时他自动打开后定位的是2

@L-x-C
Copy link
Contributor Author

L-x-C commented Jun 29, 2017

测了一下单选多选的好像都没什么问题啊...你是怎么没效果?
感觉可以改一下的是scrollIntoView的时候加个alignWithTop: true,但不知道加这个会不会和你们原有逻辑有冲突

@yesmeck
Copy link
Member

yesmeck commented Jul 4, 2017

不行哦。

screen shot 2017-07-04 at 08 44 02

我的理解 firstActiveValue="2" 的时候应该是这样?

screen shot 2017-07-04 at 08 44 52

另外没在你的修改里看到 scrollIntoView 啊,没提交?

@L-x-C
Copy link
Contributor Author

L-x-C commented Jul 4, 2017

em........我发现我现在本地也是你这样了....
就是因为他默认alignWithTopfalse,所以firstActiveValue="2"时他的2是在下拉框里的最下面。
alignWithTop没提交,因为怕会影响你们现有逻辑,所以想先问问....
不过目前来看,好像要想达到firstActiveValue的效果,alignWithTop必须设为true了。如果会影响到你们原有逻辑,那我加个判断只有在firstActiveValue的情况下才会true好了?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 96.446% when pulling 77ec980 on L-x-C:firstActiveValue into 57f1219 on react-component:master.

@L-x-C
Copy link
Contributor Author

L-x-C commented Jul 4, 2017

提交了一个配置alignWithTop的,这次应该好了,再看一下吧,麻烦了....

@yesmeck
Copy link
Member

yesmeck commented Jul 25, 2017

active 的选项应该高亮。

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 99.526% when pulling b45a39d on L-x-C:firstActiveValue into cc6f135 on react-component:master.

@L-x-C
Copy link
Contributor Author

L-x-C commented Jul 26, 2017

好了...这个coverage/coveralls为什么not successful...

@yesmeck
Copy link
Member

yesmeck commented Jul 26, 2017

rebase 下 master,然后看看是不是写个测试

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 93799dc on L-x-C:firstActiveValue into cc6f135 on react-component:master.

@yesmeck
Copy link
Member

yesmeck commented Jul 27, 2017

Great!

@yesmeck yesmeck merged commit f69a83f into react-component:master Jul 27, 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.

3 participants