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

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

Open
bisgardo opened this issue Jan 3, 2022 · 6 comments
Open

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

bisgardo opened this issue Jan 3, 2022 · 6 comments

Comments

@bisgardo
Copy link

bisgardo commented Jan 3, 2022

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
Copy link

@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
Copy link
Author

@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
Copy link

pravdomil commented Sep 19, 2022

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 added a commit to bisgardo/elm-virtual-dom-issue-178 that referenced this issue Sep 21, 2022
The code in 'src/Main.elm' is taken verbatim from pravdomil's comment 'elm/virtual-dom#178 (comment)'.
@bisgardo
Copy link
Author

bisgardo commented Sep 22, 2022

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
Copy link
Author

bisgardo commented Sep 22, 2022

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
Copy link
Author

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.

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

No branches or pull requests

3 participants