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 IndexOf function to ItemsSourceView. #2129

Merged
merged 8 commits into from
Apr 3, 2020

Conversation

marcelwgn
Copy link
Collaborator

Description

This PR adds the IndexOf function.

API spec (in progress): microsoft/microsoft-ui-xaml-specs#74

Motivation and Context

Closes #1828

How Has This Been Tested?

Not tested so far, how should this be tested?

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Mar 19, 2020
@ranjeshj ranjeshj added area-ItemsRepeater team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Mar 20, 2020
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcelwgn
Copy link
Collaborator Author

During API review @oldnewthing asked about passing null to IndexOf. We came to the conclusion that passing null should be valid. However the current implementation simply returns -1 if we pass null, regardless whether null is in the collection or not. Should I update this PR to also handle null as outlined in the API review or was there a specific reason to treat null different here?

@ranjeshj
Copy link
Contributor

During API review @oldnewthing asked about passing null to IndexOf. We came to the conclusion that passing null should be valid. However the current implementation simply returns -1 if we pass null, regardless whether null is in the collection or not. Should I update this PR to also handle null as outlined in the API review or was there a specific reason to treat null different here?

Let's wait until the api review PR is closed before making this update in case we decide otherwise.

@ranjeshj
Copy link
Contributor

ranjeshj commented Apr 1, 2020

During API review @oldnewthing asked about passing null to IndexOf. We came to the conclusion that passing null should be valid. However the current implementation simply returns -1 if we pass null, regardless whether null is in the collection or not. Should I update this PR to also handle null as outlined in the API review or was there a specific reason to treat null different here?

Let's wait until the api review PR is closed before making this update in case we decide otherwise.

@chingucoding Let's handle the null case now that we have gone through the review. Also, please merge in master to avoid the timeout issue. Thanks!

@marcelwgn
Copy link
Collaborator Author

Thanks for reminding, add test and support for null values now.

@ranjeshj
Copy link
Contributor

ranjeshj commented Apr 1, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

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

:shipit:

@kmahone
Copy link
Member

kmahone commented Apr 3, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj merged commit dc8d573 into microsoft:master Apr 3, 2020
@marcelwgn marcelwgn deleted the feat-itemssourceview-indexof branch May 15, 2020 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ItemsRepeater team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ItemsSourceView should expose IndexOf
5 participants