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

Helper line character count is ignored when TextField is created first #122

Open
matsjoyce opened this issue Aug 31, 2020 · 1 comment
Open
Labels
bug Something isn't working

Comments

@matsjoyce
Copy link
Contributor

MCVE (https://ellie-app.com/9R5Smys9fjga1):

module Main exposing (main)

import Material.TextField as TextField
import Material.HelperText as HelperText
import Material.Button as Button
import Html as H
import Browser

type alias Model = Bool
type Msg = Show

main =
    Browser.sandbox
        { init = True
        , update = update
        , view = view
        }

update msg model = not model

view model =
    let
        tf =
            case model of
                True -> H.div []
                    [ TextField.filled (TextField.config |> TextField.setMaxLength (Just 10))
                    , HelperText.helperLine [] [HelperText.characterCounter []]
                    ]
                False -> H.div [] []
    in
    H.div []
        [ Button.text (Button.config |> Button.setOnClick Show) "Show / Hide"
        , tf
        ]

When the page loads, the character count is visible. When the button is pressed twice (to hide and then show the text field) the character count disappears. This is because the second time, Elm's diffing algorithm decides to append the elements one by one, which means that the mdc-text-field element is attached to the DOM before the mdc-text-field-helper-line elements. Therefore, when the text field searches for the helper line it is not found, and the character count is not initialised.

Ideally this could be fixed by adding a reciprocal search to the helper text element, so that when it is created is searches for a preceding text field to initialise from, but that may be quite a major change. The workaround for the time being is to make the DOM before and after sufficiently different that an append is not used (e.g. add Html.map identity until you get a unique level).

@aforemny aforemny added the bug Something isn't working label Sep 1, 2020
@aforemny
Copy link
Owner

aforemny commented Sep 13, 2020

Hi @matsjoyce, thank you for reporting this! Your analysis is exactly correct, and I am aware of the following work-arounds:

  • Eliminate diffing on the common parent H.div: If in the False branch you replace the empty div with text "", you get back the insert behavior of the div as a whole that also happens when you first render the page. (Ellie)

  • Key the common parent: By keying the common parent you can achieve the same effect. (Ellie)

  • Use Html.map identity as you suggested: Thanks! I did not know this trick before. (Ellie)

I realize that this should be considered a bug in this library, and I have used the solution that you suggested in other places.

What is different about this case compared to the "select > menu > list" case is that text field and helper text components are independent of one another (both in the DOM as well as in our library), and I am not sure how I feel about that. Intuitively, I would prefer helper text functionality to be built into the text field component. I have refrained from that because it would either (conditionally) change the return type of TextField.filled from Html m to List (Html m), or would have us wrap the text field and helper text in a div. While I would favor the first approach (realized with functions filledWithHelperText), only the second approach could solve this issue. I quite dislike the extra level of nesting that this solution would impose on users.

So, maybe it would be feasible to just include the helper text component within the mdc-text-field component one way or another and have both of the benefits. I will look into the feasibility of this next.

PS. Another option that I don't know how I feel about would be to initialize helper text from the text field component (as it is now), only one animation frame later. Then we could just forget about the update order of the virtual DOM implementation (at least for this most common case.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants