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

Draft - Update TokenizingTextBox usage for PeoplePicker #51

Merged
merged 8 commits into from
Jun 12, 2020

Conversation

michael-hawker
Copy link
Member

@michael-hawker michael-hawker commented May 17, 2020

Fixes #

Update the People Picker to use the new TokenizingTextBox

PR Type

What kind of change does this PR introduce?

  • Refactoring

What is the current behavior?

Uses preview TokenizingTextBox

What is the new behavior?

Uses new TokenizingTextBox

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

TODO

TODO: Need to make PeoplePicker inherit from TokenizingTextBox instead of it's own Control?
@ghost
Copy link

ghost commented May 17, 2020

Thanks michael-hawker for opening a Pull Request! The reviewers will test the PR and highlight if there is any merge conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned Kyaa-dost May 17, 2020
@michael-hawker michael-hawker added the enhancement ✨ New feature or request label May 17, 2020
@michael-hawker
Copy link
Member Author

@azchohfi I had trouble building the WPF/XAML Islands app with this error:

2>H:\code\WindowsGraphControls\Samples\XAML Islands\WPF-Core-GraphApp\WPF-Core-GraphApp.csproj : error MSB4236: The SDK 'Microsoft.NET.Sdk.WindowsDesktop' specified could not be found.  H:\code\WindowsGraphControls\Samples\XAML Islands\WPF-Core-GraphApp\WPF-Core-GraphApp.csproj

Is there a new change I need to make to the project file?

Also, I noticed that the Microsoft.SourceLink.Vsts.Git package is marked deprecated. Should we remove that from here and the main repo?

@azchohfi
Copy link
Collaborator

Are you sure you have the Desktop dev workload installed on your VS?
About the package, we need to migrate to Microsoft.SourceLink.AzureRepos.Git.

@michael-hawker
Copy link
Member Author

@azchohfi I have these workloads installed:
image
Is there a sub-item I need to check?

TODO:
See why organizational contacts aren't showing in People Picker when connected to tenant
XAML Islands not working?
@michael-hawker
Copy link
Member Author

Alright, I think the last bit left here is to be able to fall back on the Users API as MGT does here.

Then we should be good to go.

@azchohfi I'm going to try the sub-module thing as a test for the sample app tomorrow. Then that limits the crazy NuGet stuff for now. If not I'll fall-back to the weirdness, but will I need an official NuGet reference or could I do it with MyGet still?

@azchohfi
Copy link
Collaborator

You could do it with myget, but I don't think we should. All you have to do is add the myget to the nuget.config file, but we need to remove it before we ship this to nuget.

@michael-hawker michael-hawker marked this pull request as ready for review June 12, 2020 06:30
@michael-hawker
Copy link
Member Author

Wondering if we just make the PeoplePicker a behavior and style in the future though that's not as friendly... anyway, going to merge this in and we can sort out changes later. This should be enough for the sample app.

@michael-hawker michael-hawker merged commit 6666ff3 into master Jun 12, 2020
@michael-hawker michael-hawker deleted the michael-hawker/update-tokenizing branch June 12, 2020 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants