elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Redundant removal and reinsertion of node breaks focus on element #178

Open bisgardo opened 2 years ago

bisgardo commented 2 years ago

I have a table with a variable number of columns. The right-most column of the top row contains a button element which receives focus using an initial command when the app is first loaded. Immediately after, some data is loaded, causing one unrelated column (C) to be replaced by a few others.

The td nodes of the row are rendered using Html.Keyed.node from elm/html and only the key of column C changes.

The render flow doesn't remove just column C but also the one with the focused element. Even though the element is re-inserted it no longer has the focus that was intended.

Unless this is intended behavior I'll be happy to construct a minimal example project to demonstrate the behavior. If it is intended, I'll appreciate any ideas of a clean solution to my problem.

peteygao commented 2 years ago

@bisgardo This sounds like incorrect implementation on your end.

If you want to not recreate the existing rows (ergo losing focus) when new row(s) are added, you should be using Html.Keyed.node on the tr and NOT the td nodes.

bisgardo commented 2 years ago

@peteygao No rows are inserted or removed, just columns. And the problem is not that the tds are recreated: They get reused, but get (needlessly) detached from the tree for a brief moment. This is what causes the loss of focus.

pravdomil commented 1 year ago

patching _VirtualDom_diffKeyedKids should do the trick

SSCCE

module Main exposing (..)

import Browser
import Html exposing (..)
import Html.Attributes exposing (..)
import Html.Events exposing (..)
import Html.Keyed

main : Program () Bool ()
main =
    Browser.sandbox
        { init = False
        , view = view
        , update = always not
        }

view : Bool -> Html ()
view model =
    div []
        [ Html.Keyed.node "div"
            []
            ([ input_ "A"
             , input_ "B"
             ]
                |> (\x ->
                        if model then
                            List.reverse x

                        else
                            x
                   )
            )
        , text "Type to swaps inputs."
        ]

input_ a =
    ( a
    , input
        [ id a
        , value a
        , onInput (\_ -> ())
        ]
        []
    )
bisgardo commented 1 year ago

Thanks @pravdomil for this excellent example.

The following CSS snippet makes the DOM nodes flash when they're attached, illustrating the problem even more clearly:

* {
    animation-name: flash;
    animation-duration: .5s;
}
@keyframes flash {
    from {
        background-color: green;
    }
    to {
        background-color: inherit;
    }
}
bisgardo commented 1 year ago

I also did an example that's closer to my original case in bisgardo/elm-virtual-dom-issue-178 (online at https://bisgardo.github.io/elm-virtual-dom-issue-178).

It uses the CSS trick to illustrate and shows that the bug is triggered when two (or more) consecutive elements with new keys are inserted.

bisgardo commented 1 year ago

Btw. the original case has been published (source still not public) since I filed this bug: it's the + button on https://sharesquare.xyz which gets focus on the initial load. The cell to the left of the cell holding the button is replaced by any number of cells once data finishes loading from localstorage. The key of this cell is chosen such that it works when up to 3 "participants" are loaded, but for 4 and above the focus gets lost.