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

[AutoComplete] Add key support for dataSource, add sort function property #3182

Closed
wants to merge 2 commits into from

Conversation

yongxu
Copy link
Contributor

@yongxu yongxu commented Feb 5, 2016

added key support for dataSource.
added sort function prop as well.

Closes #3137.

@mbrookes mbrookes added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Feb 5, 2016
@mbrookes mbrookes changed the title added key support for dataSource for Feature Request#3137 [AutoComplete] Add key support for dataSource, add sort function property Feb 5, 2016
@mbrookes
Copy link
Member

mbrookes commented Feb 5, 2016

@yongxu Thanks for doing this. 👍 Please could you take a look at the linting errors.

innerDivStyle={{overflow: 'hidden'}}
value={item}
primaryText={item}
/>)
Copy link
Member

Choose a reason for hiding this comment

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

Add a comma:/>),, and possibly put ), on its own line. (Not sure what the preferred house style is?)

@yongxu yongxu changed the title [AutoComplete] Add key support for dataSource, add sort function property [WIP][AutoComplete] Add key support for dataSource, add sort function property Feb 9, 2016
@yongxu
Copy link
Contributor Author

yongxu commented Feb 9, 2016

@mbrookes sorry moving took longer than expected. Just got time to work on this again, now trying to fix this asap!

@yongxu yongxu force-pushed the ac3137 branch 3 times, most recently from 6dc1f04 to a7a91fc Compare February 9, 2016 11:01
…o yongxu/ac3137

Conflicts:
	src/auto-complete.jsx
fixed issue mui#3137, removed key from last commit and resolved conflicts

NOTE:
open should not be an state, need to be fixed in the future
@mbrookes mbrookes added PR: Needs Review and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Feb 9, 2016
@yongxu yongxu changed the title [WIP][AutoComplete] Add key support for dataSource, add sort function property [AutoComplete] Add key support for dataSource, add sort function property Feb 9, 2016
@mbrookes mbrookes added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 2, 2016
@mbrookes mbrookes self-assigned this Mar 2, 2016
@mbrookes
Copy link
Member

mbrookes commented Mar 2, 2016

@yongxu - sorry this got left behind. Please could you rebase and squash, we'll get this merged before it gets more merge conflicts!

@yongxu
Copy link
Contributor Author

yongxu commented Mar 9, 2016

@mbrookes Oops, it is my bad I forgot it too. I am going to work on this rebase

@mbrookes
Copy link
Member

mbrookes commented Mar 9, 2016

Thank you @yongxu - sorry for the trouble!

@yongxu
Copy link
Contributor Author

yongxu commented Mar 11, 2016

@mbrookes Looks like this PR is too hard to rebase. I am going to rewrite some of the changes and start another PR that includes all the updates from this one. Will finish either tonight or tomorrow :)

@yongxu yongxu closed this Mar 11, 2016
yongxu added a commit to yongxu/material-ui that referenced this pull request Mar 11, 2016
@yongxu
Copy link
Contributor Author

yongxu commented Mar 11, 2016

The rewritten PR is finished, please checkout #3662

yongxu added a commit to yongxu/material-ui that referenced this pull request Mar 13, 2016
yongxu added a commit to yongxu/material-ui that referenced this pull request Mar 13, 2016
yongxu added a commit to yongxu/material-ui that referenced this pull request Mar 13, 2016
yongxu added a commit to yongxu/material-ui that referenced this pull request Mar 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants