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

checkbox re-render is forever checked #405

Closed
matiu2 opened this issue Mar 26, 2020 · 5 comments · Fixed by #406
Closed

checkbox re-render is forever checked #405

matiu2 opened this issue Mar 26, 2020 · 5 comments · Fixed by #406
Labels
bug Something isn't working

Comments

@matiu2
Copy link

matiu2 commented Mar 26, 2020

If you render a input![attrs!["type" => "checkbox", "checked" => "checked"]] then after some event, re-render it without the the "checked" => "checked" it looks good in the DOM inspector, but on the screen it's still checked.

To get around it, I guess I have to give every checkbox an ID and call JS to uncheck it.

tested in brave browser, which is chrome based.

@MartinKavik
Copy link
Member

MartinKavik commented Mar 26, 2020

Thanks for the report.
Google says this issue is quite popular among front-end frameworks thanks to attributes/properties madness*.
Resolved in the referenced PR above.

madness* - copy/paste from Elm docs:

So the class attribute corresponds to the className property. At first glance, perhaps this distinction is defensible, but it gets much crazier. There is not always a one-to-one mapping between attributes and properties! Yes, that is a true fact. Sometimes an attribute exists, but there is no corresponding property. Sometimes changing an attribute does not change the underlying property. For example, as of this writing, the webkit-playsinline attribute can be used in HTML, but there is no corresponding property!

or copy/paste from Vaadin docs:

Be cautious when using attributes and properties:

  • In many cases it is possible to use either an attribute or property with the same name for the same effect, and both work fine.
  • However, in certain cases:
    • Only one or the other works, or
    • The attribute is considered only when the element is initialized, and the property is effective after initialization.

You should always check the specific documentation for the element you’re using to find out whether a feature should be configured using a property or an attribute.

@matiu2
Copy link
Author

matiu2 commented Mar 27, 2020

In case anyone else comes across this and needs some guidance. My use case is I have a:

<span><input type=checkbox> bunch of other stuff </span>

and I've made the outer span clickable, so in the view, when I grab the click event, I toggle the checkbox, as well as sending my Msg to update the model:

use seed::prelude::*;
use wasm_bindgen::JsCast;

... 
            // Events
            ev(Ev::Click, {
                let sso = sso.clone();
                move |evt| {
                    // Because browsers don't have a consistency with attributes and properties, we
                    // have to set the property of the checkox here
                    // We can't *just* set attrs!["checked"..] We have to run some js to make it
                    // work properly
                    // See: https://github.com/seed-rs/seed/issues/405#issuecomment-604324286
                    let checkbox = evt
                        .target()
                        .and_then(|ele| ele.dyn_into::<web_sys::HtmlElement>().ok())
                        .and_then(|ele| ele.first_element_child())
                        .and_then(|ele| ele.dyn_into::<web_sys::HtmlInputElement>().ok());
                    match checkbox {
                        Some(checkbox) => checkbox.set_checked(!checkbox.checked()),
                        None => log!(format!("Unable to grab the checkbox for: {}", &sso)),
                    };
                    Msg::ToggleEmployee(sso)
                }
            }),

I should probably do the actual change in the update. I don't really know what I'm doing, but I'm hacking my way through :) Machete programming.

@MartinKavik
Copy link
Member

Standard way (should work on released Seed versions):

    span![
        input![
            attrs!{At::Type => "checkbox"},
            attrs!{At::Checked => model.checked.as_at_value()}
        ],
        "bunch of other stuff",
        ev(Ev::Click, |_| Msg::Toggle)
    ]

Alternative way with direct DOM manipulation (not recommended):

    span![
        input![
            el_ref(&model.my_checkbox),
            attrs!{At::Type => "checkbox"},
        ],
        "bunch of other stuff",
        ev(Ev::Click, {
            let my_checkbox = model.my_checkbox.clone();
            move |_| {
                let my_checkbox = my_checkbox.get().expect("my_checkbox DOM element");
                let checked = my_checkbox.checked();
                my_checkbox.set_checked(not(checked));
                Msg::Toggle
            }
        })
    ]

Future way

    span![
        input![
            A.type_().checkbox(),
            A.checked(model.checked),
        ],
        "bunch of other stuff",
        E.click(|_| Msg::Toggle),
    ]

@matiu2
Copy link
Author

matiu2 commented Mar 28, 2020

Thank you Martin, that has moved me forward.

What about "indeterminate" state ? https://css-tricks.com/indeterminate-checkboxes/

I'm guessing at the moment, the only way will be with direct DOM manipulation or JS ?

@MartinKavik
Copy link
Member

@matiu2 See #408.

  • It's a custom element to encapsulate DOM manipulation and to make Rust HTML template declarative.
  • Rust custom elements are planned - Web Components support #336.
  • There are multiple JS/TS custom elements in our Seed projects (e.g. TS custom element for code highlighting on seed-rs.org).
  • Less declarative alternative would be React-like Hooks (in development).
  • And yes, you can write it also in Elm architecture but I think it's not the best option.

I suggest to continue on https://seed.discourse.group/

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

Successfully merging a pull request may close this issue.

2 participants