Implement RichSuggestBox

[huynhsontung]

Fixes

Closes #3649

Implement rich suggest text control as discussed in the issue.

PR Type

What kind of change does this PR introduce?

• Feature

What is the current behavior?

None. The control did not exist before this PR.

What is the new behavior?

'RichSuggestBox' is now a usable control in the Microsoft.Toolkit.Uwp.UI.Controls namespace.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link: Documentation for RichSuggestBox control MicrosoftDocs/WindowsCommunityToolkitDocs#558
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• 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

Other information

Convenient issue tracker

• Limited styling options for tokens. Only text styling options are available.
• Tokens are raw text and not UI elements. UI events (e.g. UIElement.PointerEntered) are not available. This makes behaviors like showing flyout when hover mouse over the token impossible.
• Tokens are always underlined.
• Tokens cannot adapt to theme change dynamically. They will lose foreground color while having the same background color which will most likely introduce bad contrast.
• Cannot disable spelling check on tokens (token text may have red underline).
• Cannot use Tab to select a suggestion due to conflict with the accessibility feature.
• Deleted tokens will be kept in an internal list until cleared programmatically to support Undo and Redo.

RichEditBox issues

Below are RichEditBox related issues encountered during the development of this control.

• Getting Link property from a range might change the range.
• Replacing a formatted link at the beginning of the document does not reset the formatting. The replaced text still has the same formatting.
• Setting the Link property can sometimes fail but only when the target text is at the beginning of the document.
• There is no event for when text is about to change. TextChanging and SelectionChanging are invoked after the text has changed.
• Character format foreground color cannot display a range of colors (e.g. #F7FFB3)

[ghost]

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

[net-foundation-cla]

All CLA requirements met.

[net-foundation-cla]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ huynhsontung sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[michael-hawker]

Thanks @huynhsontung for the PR! Can you please run the UpdateHeaders.bat or '.\build.ps1 -target=UpdateHeaders' and commit the changes? This should get to the next stage in the CI to ensure everything can build and setup a preview package folks can more easily test out.

I've put this in the 7.1 milestone for now, so it may take us a bit of time to get to reviewing in more depth.

[michael-hawker]

@huynhsontung I'm not sure what happened with your Update Headers but it touched like every file in the toolkit... Can you revert the head of your branch, re-commit only the required headers and force-push back? Thanks!

There's also some StyleCop issues to resolve, see: https://dev.azure.com/dotnet/WindowsCommunityToolkit/_build/results?buildId=44049&view=logs&j=4a47a688-b50e-58f6-4d2c-6d1b829fc8e2&t=503927fd-5045-588b-b5d5-40c5f89cda06&l=10755

[huynhsontung]

@michael-hawker I was wondering why the script changed every file in the repo. Does the script check for line ending? I might have auto CRLF misconfigured.

I will resolve all the StyleCop issues at once.

[huynhsontung]

@michael-hawker I have resolved all the build issues. I also added a sample page for easy testing. Let me know what you think.

Features still need to be worked on at the moment:

• Clear tokens on copy/cut.
• Floating suggestion dropdown instead of bottom-edge dropdown style like AutoSuggestBox.

[Kyaa-dost]

@huoyaoyuan I can see that this PR is still being worked on but I have tried testing the sample and I have noticed that whenever the text in the field is highlighted from right to left, part of the font box cuts off.

Again I know this might not be the final version but just wanted to provide some feedback before the PR is ready to be tested.

[michael-hawker]

Thanks @huynhsontung for fixing up the issues so a build passes the CI! It makes it a lot easier for folks to pull down and play with! We're still heads-down on wrapping our 7.0 release at the moment, so it might take me a bit of time to dive in for more detailed feedback. I'm excited to check this control out though! 🙂

In the meantime it seems @Kyaa-dost has been helping to check things out, so let us know when your remaining issues are cleared up and he can help validate them as well.

[michael-hawker]

@huynhsontung starting to get back into things now that we've shipped 7.0. Haven't dug too deep into this yet, but have a couple of quick initial comments. It also has to be moved to the Input folder/project now that we've split up the controls package (basically fast-forward to our current changes). Maybe easiest to rebase?

We should probably think a bit about how we could test this type of control as well. We'll be trying to figure out in the next couple of weeks how we manage the bar for tests and have different avenues based on that. Should know more soon to share, but just wanted to give you a heads up.

Excited to test this out more as it's certainly an experience commonly seen on the web and in many social apps these days! 🎉

[huynhsontung]

@michael-hawker thank you for letting me know. As I have been incredibly busy lately, I won't be able to work on these changes for the next week or two. I will definitely look into it soon after 🚀

[huynhsontung]

@michael-hawker I have moved the control to the Input project. Following up on your comments, I unregistered all events before registering and referenced WinUI style directly instead of using a copy.

I also discovered some behaviors of RichEditBox that may interfere with how tokens work, like formatting after removing formatted text. I will look into it and find workarounds for unwanted behaviors.

Regarding testing, I have not had much experience with UI testing myself. Will be interesting to know more about how WCT tests UI.

[huynhsontung]

As partially outlined in the issue, there are some shortcomings with the current RichSuggestBox that I can't really find a solution for:

• There seems to be no way to remove the token's underline.
• No way to disable proofing check on the token text.
• Cannot use Tab key to select a token, since it is conflicted with Windows' navigation. I opted to use Ctrl + Tab instead.

That being said, I am pretty happy with how RichSuggestBox turned out. I will be focusing on writing tests for the control from now on. The control does have a lot of edge case handling.

[michael-hawker]

@huynhsontung are the unit tests running locally for you? Seems like one of the TokenizingTextBox tests is suddenly failing in the CI here? Not sure if there's a naming conflict in the style somewhere for the RichSuggestBox? That's the only thing I could think of that could influence the other test?

[michael-hawker]

@huynhsontung glad you figured out the issue. Let us know if you need some guidance on writing unit tests or UI Interaction tests (though we're waiting for #4007 to be merged to resolve some issues with that).

In the meantime, we'll take a look at the corner cases. They don't seem to be too monumental, but will try it out and provide more feedback.

Thanks!

[michael-hawker]

FYI @huynhsontung I have a PR open with info on our whole testing setup: CommunityToolkit/WindowsCommunityToolkit-wiki#33 let me know if you have any questions.

Looks like there's a merge conflict from something now, so it may be good to rebase the PR?

[huynhsontung]

I have opened a PR for a RichSuggestBox docs page. Please find it here MicrosoftDocs/WindowsCommunityToolkitDocs#558

[michael-hawker]

Last two remaining items I'm aware of are:

• Question on XAML Root
• Dispose vs. Cancel for cleaning up the CancellationTokenSource?

[michael-hawker]

Looked up the docs, so looks TransformToVisual(null) is doing what we want already (getting coordinates from the tree root). You should grab the bounds the same way you did in IsElementInsideWindow, checking Window.Current null above doesn't seem right?

[huynhsontung]

We want the window position relative to the top left of the screen, not to the tree root. I don't know if I can get absolute screen coordinates without using Window.

edit: From Window class docs, Window.Current is set for UWP and null for Desktop apps. If Window.Current is null we can just assume the element is on-screen.

[michael-hawker]

See the docs on the migration here.

I think the missing thing this guide doesn't show is how to get the proper X, Y coordinates as you're using that from the window bounds. @marb2000?

[huynhsontung]

WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI.Controls.Input/RichSuggestBox/RichSuggestBox.Helpers.cs

Lines 21 to 29 in 3401db0

	private static bool IsElementOnScreen(FrameworkElement element, double offsetX = 0, double offsetY = 0)
	{
	// DisplayInformation only works in UWP. No alternative to get DisplayInformation.ScreenHeightInRawPixels
	// Tracking issues:
	// https://github.com/microsoft/WindowsAppSDK/issues/114
	// https://github.com/microsoft/microsoft-ui-xaml/issues/4228
	// TODO: Remove when DisplayInformation.ScreenHeightInRawPixels alternative is available
	return true;

As previously discussed, I have disabled the element on screen check due to not being able to get the screen size in a desktop app (DisplayInformation only works in UWP). As a result, the suggestion popup no longer opens upward when RichSuggestBox moves too close to the bottom edge of the screen. The popup still opens upward when RichSuggestBox is too close to the bottom edge of the current window.

Tracking issues:
microsoft/WindowsAppSDK#114
microsoft/microsoft-ui-xaml#4228

[ghost]

Hello @michael-hawker!

Because this pull request has the auto merge :zap: label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[michael-hawker]

Thanks so much @huynhsontung for creating this amazing control! It's super exciting to have this experience in the Toolkit now for everyone! 🎉🎉🎉