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.

Sign up for GitHub

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

Jump to bottom

Porting TokenizingTextBox #77

Merged
merged 30 commits into from
Jul 11, 2023
Merged

Porting TokenizingTextBox #77

merged 30 commits into from
Jul 11, 2023

Conversation

niels9001
Copy link
Collaborator

@niels9001 niels9001 commented Jun 3, 2023 •
edited
Loading

This PR ports the TokenizingTextBox to the new repo, addressing: #53

Changes:

  • Ported over code, sample and tests.
  • Visual update to the TokenizingTextBox to make it inline with WinUI styling:

Before:
image

After:
image

  • Working on WASDK and UWP - couldn't figure out why it's not showing the tokens on WASM. Might be the ItemsPresenter issue we were having with SettingsExpander as well?

@niels9001 niels9001 requested a review from Arlodotexe June 7, 2023 12:24
@niels9001 niels9001 marked this pull request as ready for review June 7, 2023 12:24
@niels9001
Copy link
Collaborator Author

@Arlodotexe When copying the WrapPanel code manually into the project, everything works. When referencing the .Primitives.proj I'm getting this error:

image

@niels9001 niels9001 linked an issue Jun 7, 2023 that may be closed by this pull request
Port TokenizingTextBox #53
Closed
@niels9001
Copy link
Collaborator Author

@Arlodotexe did you see the crash as mentioned above?

@Arlodotexe
Copy link
Member

@Arlodotexe did you see the crash as mentioned above?

It looks like you are referencing the primitives .csproj directly via the MSBuild property ToolkitPrimitiveSourceProject, but I'm not seeing the crash you mentioned on any platform.

Uno does appear to have an issue though. We should file a ticket to investigate this issue so it can be fixed from Uno's end.

michael-hawker
michael-hawker reviewed Jul 6, 2023
View reviewed changes
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Didn't try testing locally yet, but just going through a few things to get up to speed. Did a comparison with Uno fork and didn't notice any big differences. I know originally, I had tested this control on Android for the Graph components, but don't think I had tried WASM at the time.

components/TokenizingTextBox/samples/TokenizingTextBox.md Outdated Show resolved Hide resolved
components/TokenizingTextBox/src/CommunityToolkit.WinUI.Controls.TokenizingTextBox.csproj Show resolved Hide resolved
components/TokenizingTextBox/src/TokenizingTextBox.Selection.cs Outdated Show resolved Hide resolved
components/TokenizingTextBox/src/InterspersedObservableCollection.cs Outdated Show resolved Hide resolved
@niels9001 @michael-hawker
Update components/TokenizingTextBox/src/InterspersedObservableCollect…
34e8ab6 
…ion.cs

Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com>
@Arlodotexe Arlodotexe mentioned this pull request Jul 10, 2023
Closed
@Arlodotexe Arlodotexe merged commit 56dff69 into main Jul 11, 2023
7 checks passed
@delete-merged-branch delete-merged-branch bot deleted the niels9001/TokenizingTextBox branch July 11, 2023 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arlodotexe Arlodotexe Arlodotexe approved these changes

@michael-hawker michael-hawker michael-hawker left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Port TokenizingTextBox
3 participants
@niels9001 @Arlodotexe @michael-hawker