fable-compiler / Fable.Lit

Write Fable Elmish apps with Lit
https://fable.io/Fable.Lit/
MIT License
91 stars 13 forks source link

Hooks.useTransition is leaving transition-enter classname to its next sibling. #45

Closed leolorenzoluis closed 2 years ago

leolorenzoluis commented 2 years ago

So I implemented it and it almost is identical to the animations that you have for TodoMVC. However, when I try to delete an item from the list say in the middle, then the next item will add transition-enter on the next sibling. It looks like transition-enter is getting applied to the next item after transition.triggerLeave is called?

Given the following code:

[<HookComponent>]
let Monitor (item: Monitor) dispatch =
    let transitionMs = 200
    let transition = Hook.useTransition(transitionMs, onLeft = (fun () -> DeleteMonitor item.Id |> dispatch))
    let className = Hook.use_scoped_css $"""
        :host {{
            transition-duration: {transitionMs}ms;
            border: 2px solid lightgray;
            border-radius: 10px;
            margin: 5px 0;
        }}
        :host.transition-enter {{
            opacity: 0;
            transform: scale(2);
        }}
        :host.transition-leave {{
            opacity: 0;
            transform: scale(0.1);
        }}
        .is-clickable {{
            user-select: none;
        }}
    """
    html $"""
    <div class="{className} {transition.className}" {LitLabs.motion.animate()}>
        <div>
            <p>{item.Id} {item.FriendlyName} {item.URL}</p>
            <sl-button @click={Ev (fun _ -> transition.triggerLeave())}>Delete monitor</sl-button>
            <sl-button @click={Ev (fun _ -> EditMonitor item |> dispatch)}>Edit monitor</sl-button>
        </div>
    </div>
    """

[<HookComponent>]
let Page() =
    let model, dispatch = Hook.useElmish(init, update)
    let renderItem (item: Monitor) =
        match model.Edit with
        | Some value when value.Id = item.Id->
            html $"""
            <div>
                <p>{item.Id}</p>
                <input value={item.FriendlyName} @keyup={EvVal (fun e -> UpdateFriendlyName e |> dispatch)}/>
                <input value={item.URL}/>
                <sl-button @click={Ev (fun e -> SaveMonitor model.Edit.Value |> dispatch)}>Save monitor</sl-button>
            </div>
            """
        | _ ->
            Monitor item dispatch

    html $"""
     <sl-button @click={Ev (fun e ->
            let newMonitor = {
                Id = Random.Shared.Next() 
                FriendlyName = "Test1"
                URL = "2"
                Retries = 100
                HeartBeatInterval = 100
            }
            CreateMonitor newMonitor |> dispatch)}>New monitor</sl-button>
        {model.Monitors |> Seq.map renderItem}
    """

Screen Shot 2022-07-05 at 12 14 46 AM

When the delete button is clicked, the next sibling is left hanging with transition-enter. I expect no transition class name is left hanging. I've checked the update function that I have and the delete is only getting called once.

Update: I changed model.Monitors |> Seq.map to Lit.mapUnique and it works perfectly fine. Documentation refer to the method as

mapUnique You can pass any iterable (including F# lists) to Lit, but when it's important to identify items in a list (e.g. when the list can be sorted, filtered or included item transitions), use Lit.mapUnique to give each item a unique id.

Not sure what included item transition means here and why would it fix it? Also, why does mapUnique expects the Id to be a string?

alfonsogarciacaro commented 2 years ago

This is probably due to how Lit does the diffing when removing/adding elements from a list. Apparently it's much faster to remove the last child instead of one in the middle. So what Lit does is, instead of deleting from the middle of the list, it removes the actual HTML element and the end and reuses the other elements. This is why the hook gets confused and applies the class to what should be a different element.

To avoid this, when rendering a dynamic list, it's recommended to use mapUnique to give a unique ID to each item. This will allow Lit to correctly identify each item and avoid situations like this: https://github.com/fable-compiler/Fable.Lit/blob/6317d3a32ed91927984b1fc03aa7da3fc02affa3/src/Lit/Lit.fs#L265-L272

leolorenzoluis commented 2 years ago

Lit expects a string as its identifier?

alfonsogarciacaro commented 2 years ago

Yes, this is common in JS (React also expects the key prop to be a string). Keys must be unique and shouldn't change between renders (e.g. don't use the index if you're going to sort the list or remove/add elements in the middle).

alfonsogarciacaro commented 2 years ago

With Fable a usual pattern is to add a Guid property to the items and then convert it to string when passing to Lit or React.