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

Add gestureRecognizer shortland modifier #39

Merged
merged 4 commits into from
Jun 1, 2023

Conversation

edgarfgp
Copy link
Member

Span("Hello")
    .font(size = 14.)
    .gestureRecognizer(TapGestureRecognizer(TapCommand))

@TimLariviere
Copy link
Member

I would be very careful with this kind of shorthand.

Having 2 modifiers to do the same thing (gestureRecognizers vs gestureRecognizer) could lead to unwanted behavior and crashes, because Fabulous will treat those 2 modifiers as independants even though they both work with the same collection behind the scene.

So doing this would be possible:

Label("Hello")
    .gestureRecognizer(TapGestureRecognizer(Tapped))
    .gestureRecognizers() {
        DragGestureRecognizer(Dragged)
    }

As a user, I would expect both gestures to be applied, but most likely Fabulous will just crash because it expects 1 property = 1 modifier exclusively to be able to do diffing correctly.

In this case, when applying Tapped, Fabulous expects the Gestures collection to be empty but it already contains Dragged. Most likely, this will result in a IndexOutOfRange at some point.

@TimLariviere
Copy link
Member

TimLariviere commented May 30, 2023

If you really wish to have this kind of shorthand modifier, you need to keep a single AttributeDefinition for all variants of the same modifier. Like this Fabulous will know it's actually the same modifier and will work with only the "latest" one declared.

eg.

type ViewModifiers =
    // The main one that every other variants will call instead of creating their own attributes
    static member gestureRecognizers(this: WidgetBuilder<#IFabView>) =
        AttributeCollectionBuilder(...)

type ViewExtraModifiers =
    // The shorthand
    static member gestureRecognizer(this: WidgetBuilder<#IFabView>, gesture: WidgetBuilder<#IFabGestureRecognizer>) =
        this.gestureRecognizers() { gesture }
// Dragged is the latest one, Fabulous will ignore Tapped
Label("Hello")
    .gestureRecognizer(TapGestureRecognizer(Tapped))
    .gestureRecognizers() {
        DragGestureRecognizer(Dragged)
    }

// Tapped is the latest one, Fabulous will ignore Dragged
(Label("Hello")
    .gestureRecognizers() {
        DragGestureRecognizer(Dragged)
    })
    .gestureRecognizer(TapGestureRecognizer(Tapped))

@edgarfgp edgarfgp force-pushed the gestureRecognizer-shortland-modifier branch from 40e199b to 963dd3d Compare May 30, 2023 10:18
@edgarfgp
Copy link
Member Author

@TimLariviere This is ready

@edgarfgp edgarfgp added this pull request to the merge queue Jun 1, 2023
Merged via the queue into main with commit c56f09c Jun 1, 2023
1 check passed
@edgarfgp edgarfgp deleted the gestureRecognizer-shortland-modifier branch June 1, 2023 09:23
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