-
Notifications
You must be signed in to change notification settings - Fork 477
Conversation
XamarinCommunityToolkitSample/ViewModels/Behaviors/TagBehaviorViewModel.cs
Outdated
Show resolved
Hide resolved
XamarinCommunityToolkitSample/Pages/Behaviors/TagBehaviorPage.xaml
Outdated
Show resolved
Hide resolved
@rdelrosario very good work with comments! Thanks for the quick updates 👍 Just a few things which we should update, and then we will get merged this awesome feature :) |
Also, for next ones please add it directly. In this startup phase it will be OK. Somewhere in the future we will want PRs to be complete. It's not a complete feature until it has all the bits: the feature, tests and a sample. If tests cannot be created for this, that's fine too! Just note that and why you think it doesn't make sense/why it's not possible. Especially for UI heavy things like these that is perfectly possible. |
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Gerald Versluis <github@geraldversluis.nl>
Co-authored-by: Gerald Versluis <github@geraldversluis.nl>
…xaml Co-authored-by: Gerald Versluis <github@geraldversluis.nl>
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Andrei <andrei.misiukevich@gmail.com>
Co-authored-by: Gerald Versluis <github@geraldversluis.nl>
Hey @jfversluis and @AndreiMisiukevich, @IeuanWalker will be extending this PR this will add more functionality, making it more generic and extensible since he has an issue #178 that's related to this. Btw @IeuanWalker, let's rename this to MatchBehavior since now will be more generic not just tags. |
@IeuanWalker Actually this is a Xamarin Forms issue when having Spans in a Label with multiple lines. xamarin/Xamarin.Forms#11650 @jfversluis how should we proceed with this? |
# Conflicts: # XamarinCommunityToolkit/Behaviors/MaskedBehavior.shared.cs # XamarinCommunityToolkitSample/Resx/AppResources.Designer.cs # XamarinCommunityToolkitSample/Resx/AppResources.resx
Could someone summarize what the current status is please? :) Do I understand correctly that this feature is now done, but it shows some weird behavior because of a Forms bug? What bug is that? |
@jfversluis The feature is done, but there is an issue when using it with multiple lines in Android due to a Xamarin Forms issue with Label - Span (xamarin/Xamarin.Forms#11650) |
# Conflicts: # XamarinCommunityToolkitSample/Resx/AppResources.es.Designer.cs # XamarinCommunityToolkitSample/Resx/AppResources.es.resx # XamarinCommunityToolkitSample/Resx/AppResources.resx # XamarinCommunityToolkitSample/ViewModels/Behaviors/BehaviorsGalleryViewModel.cs
Hm I'm kind of conflicted on this one. I very much like it, but if this means we will get issues from it from a known bug then I don't know if we should do it. If I understand this correctly if you use more than 1 clickable label this bug will happen right? So at this point it's basically unusable for more than 1 tappable object? But only on Android? |
Right, on iOS everything works as expected. But when using multiline labels
in Android then you get the issue when you tap.
…On Thu, Sep 3, 2020, 3:30 AM Gerald Versluis ***@***.***> wrote:
Hm I'm kind of conflicted on this one. I very much like it, but if this
means we will get issues from it from a known bug then I don't know if we
should do it.
If I understand this correctly if you use more than 1 clickable label this
bug will happen right? So at this point it's basically unusable for more
than 1 tappable object? But only on Android?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#172 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AATOAJ5WCYUL3UJNE6ETASDSD5AZVANCNFSM4PO4SUFA>
.
|
In addition to the android issue, I also think we need to move the command parameter down to the MatchType so each type of click can be handled separately. This is what it currently looks like - <Label.Behaviors>
<behaviors:MatchBehavior Command="{Binding TagTappedCommand}">
<behaviors:MatchBehavior.MatchTypes>
<behaviors:HashtagMatchType/>
<behaviors:MentionMatchType />
<behaviors:HtmlLinkMatchType />
</behaviors:MatchBehavior.MatchTypes>
</behaviors:MatchBehavior>
</Label.Behaviors> Where as if we could get it like this it would prevent the user from adding logic to work out the type of label that was clicked - <Label.Behaviors>
<behaviors:MatchBehavior>
<behaviors:MatchBehavior.MatchTypes>
<behaviors:HashtagMatchType Command="{Binding HashtagTappedCommand}"/>
<behaviors:MentionMatchType Command="{Binding MentionTappedCommand}"/>
<behaviors:HtmlLinkMatchType Command="{Binding HtmlLinkTappedCommand}"/>
</behaviors:MatchBehavior.MatchTypes>
</behaviors:MatchBehavior>
</Label.Behaviors> I've tried to change it my self but couldn't get it working :/ |
@IeuanWalker @jfversluis I can do the command support for each MatchType change, but if this PR won't go forward because of the XF Android issue, it might not be worth it. So please let me know how to proceed. |
I very much like this feature, but I think it's kind of unusable until that bug in Forms is fixed? Would you agree? If so, I think we should wait until that is fixed and then add it. I think else we will just confuse people by knowingly adding things in here that don't work and then saying: sorry, this is a bug in Forms and keep issues around here as well until it's fixed there. |
@jfversluis Yes, I agree. |
As agreed, as long as this issue in Forms xamarin/Xamarin.Forms#4143 still exists this is not something we want to take in, foreseeing all the work and issues we would probably get from it. I'm sorry @rdelrosario :( Closing this for now, hopefully we can still get it in, in the future. |
@rdelrosario could you please properly link this PR to the mentioned issues? (even if it is inactive for now), in case of doubt on how to https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword |
Linked issues |
I made a PR to fix xamarin/Xamarin.Forms#4143 at least for Android: xamarin/Xamarin.Forms#13348 |
@Braini01 that's awesome, thanks :) |
@jfversluis My Xamarin Forms PR was merged and is part of Xamarin.Forms 5.0.0.2083 (5.0.0 Service Release 4). So this PR could be reevaluated? |
Description of Change
Added MatchBehavior.
Related to issue #171 and #178
HOW IT WORKS
The MatchBehavior can be added to Xamarin Forms Label Control, it contains the following properties:
MatchTypes: Specifies the match types that we want to detect and style.
Command: Specifies the command that will be executed after tapping on any detected match type.
Usage
Example:
Note: The text must be set in the FormattedText property for this behavior to work
Demo:
PR Checklist
Note: Will add the unit testing in a different PR