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

Update Touch Behavior Sample #1784

Merged
merged 6 commits into from
Apr 28, 2024

Conversation

Axemasta
Copy link
Contributor

@Axemasta Axemasta commented Apr 1, 2024

Description of Change

Update the TouchBehavior sample gallery to show the recommended method of setting of setting the binding context for the TouchBehavior.

I have included a new usage with the CommandParameter to help clearup some confusion we've been seeing for people consuming this Behavior upon release.

Linked Issues

N/A

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Docs PR

Additional information

I've run dotnet format and checked that in, some minor changes all in the TouchBehavior area.

Screenshots of changes to the sample:
Command parameter sample with no picker selection
Command parameter sample after command executed with no picker selection
Command parameter sample after command executed with no picker selection

@Axemasta Axemasta marked this pull request as ready for review April 1, 2024 13:23
@brminnick
Copy link
Collaborator

@Axemasta Could you resolve the merge conflicts?

It looks like the work we did on #1791 caused a couple conflicts on this PR.

@Axemasta
Copy link
Contributor Author

Axemasta commented Apr 5, 2024

@Axemasta Could you resolve the merge conflicts?

It looks like the work we did on #1791 caused a couple conflicts on this PR.

Yes I was waiting for your changes before updating this PR, all resolved now!

Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Thanks @Axemasta! It looks like the LongPress | Hover section is broken in this update.

Could you fix that? And then we'll merge it!

The sample now shows the recommended way to set the binding context for the touch behavior
Ran dotnet format on the solution, looks like its touch behaviour stuff that needed the formats!
There is now an example of the touch behavior being used to pass a command parameter
Seperate the sections out into more clearly seperated segments for visual clarity
This change set got quite messy when rebasing so I've now resolved everything by hand
@Axemasta Axemasta force-pushed the touch-behavior-sample branch 2 times, most recently from e17e29d to 977dd34 Compare April 28, 2024 21:32
Somehow managed to remove this... 🙈
@Axemasta
Copy link
Contributor Author

@brminnick I've pushed a fix & rebased ontop of main 👍

@brminnick
Copy link
Collaborator

Wonderful! Thank you sir 🫡

@brminnick brminnick merged commit 46a7dd8 into CommunityToolkit:main Apr 28, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants