stefan-hoeck / idris2-dom-mvc

Single Page Web Applications in Idris
BSD 3-Clause "New" or "Revised" License
19 stars 2 forks source link

Checkbox state needs to be updated via property instead of attribute #33

Closed gergoerdi closed 1 year ago

gergoerdi commented 1 year ago

Please see https://github.com/gergoerdi/idris2-dom-mvc-issue-33 for a full reproducer.

It consists of two controls: a checkbox and a button. The state is a single Bool, displayed by the checkbox. Checking/unchecking the checkbox, or clicking the button, flips the state.

As long as the toggle button is clicked, the state changes as expected and the checkbox reflects said change. However, as soon as the user clicks on the checkbox, later state changes triggered by the toggle button are NOT reflected in the checkbox state.

gergoerdi commented 1 year ago

Looking at the DOM in Chrome's inspector after "breaking" it (i.e. when the checkbox doesn't reflect the underlying state anymore), I see the checked attribute appearing/disappearing from the checkbox, but visually its state doesn't change.

Perhaps relevant, this quote from https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#additional_attributes (emphasis mine):

checked

A boolean attribute indicating whether this checkbox is checked by default (when the page loads). It does not indicate whether this checkbox is currently checked: if the checkbox's state is changed, this content attribute does not reflect the change. (Only the HTMLInputElement's checked IDL attribute is updated.)

gergoerdi commented 1 year ago

Doing some experiments in Chrome's console, starting from a fresh page load (i.e. I haven't clicked the checkbox yet):

> const box = document.getElementById("checkbox");

> box.getAttribute("checked") 
< null

> box.setAttribute("checked", "") 
# At this point, the checkbox becomes checked in the UI
> box.getAttribute("checked")
< ""

> box.removeAttribute("checked") 
# At this point, the checkbox becomes unchecked in the UI
> box.getAttribute("checked")
< null

If I click on the checkbox, and try the above sequence again, then although the "checked" attribute reflects the changes, the UI does not change.

However, if instead I try changing the checked property, then the UI reflects the changes:

> box.checked
< false

> box.checked = true
# The UI shows checkmark
> box.checked
< true

> box.checked = false
# The UI is now unchecked
> box.checked
< false

So basically instead of setting the "checked" attribute, dom-mvc needs to change the checked property.

stefan-hoeck commented 1 year ago

This is similar to what we already to with value for <input> elements: You can set the initial value via an attribute but later changes to the value being displayed need to go via the value property. See Web.MVC.View.value for the corresponding command. Something similar to what we do there will be needed for check boxes.

gergoerdi commented 1 year ago

I tried setting checked via HTMLInputElement but that might have uncovered another bug:

checked : Ref Tag.Input -> Bool -> Cmd ev
checked r b = C $ \h => castElementByRef r >>= (HTMLInputElement.checked =. b)

This typechecks, but then results in a runtime JS type error:

Uncaught TypeError: $2 is not a function
    at JS_Attribute_fromPrim (main.js:1075:76)
    at Web_Raw_Html_HTMLInputElement_checked (main.js:582:9)
    at main.js:562:242
    at JS_Attribute_x3dx2e (main.js:1086:26)
    at main.js:562:215
    at main.js:843:141
    at Prelude_Types_either (main.js:656:34)
    at main.js:843:47
    at main.js:453:10
    at x24tcOpt_6 (main.js:402:30)

(in the given frame, $2's value is the string "HTMLInputElement.getchecked")

stefan-hoeck commented 1 year ago

I'll need some more context for what you wrote above, as your implementation of checked works fine on my end. Are you using an unsafe cast somewhere else in your code? The string "HTMLInputElement.getchecked" is an error message from the dom library, coming from trying to convert a non-bool to a Bool via fromFFI. That should not happen here unless something very strange is going on.

gergoerdi commented 1 year ago

I've pushed a commit to the repro repo that gives me this error, can you look at that?

stefan-hoeck commented 1 year ago

I've pushed a commit to the repro repo that gives me this error, can you look at that?

I clone your repo, built it with make page and checked html.index in Firefox and Chromium. Both seem to work fine without error.

gergoerdi commented 1 year ago

I don't know what to say... Did you try clicking the button and watching the console?

Do you have the following in your compiler output?

function Web_Raw_Html_HTMLInputElement_checked($0) {
 return JS_Attribute_fromPrim($4 => JS_Boolean_toFFI_ToFFI_Bool_Boolean($4), $8 => JS_Boolean_fromFFI_FromFFI_Bool_Boolean($8), 'HTMLInputElement.getchecked', $d => $e => Web_Internal_HtmlPrim_HTMLInputElement_prim__checked($d, $e), $13 => $14 => $15 => Web_Internal_HtmlPrim_HTMLInputElement_prim__setChecked($13, $14, $15))($0);
}
stefan-hoeck commented 1 year ago

Yes, I checked the console. Nothing there. Interestingly, my checked function is just slightly different:

function Web_Raw_Html_HTMLInputElement_checked($0) {
 return JS_Attribute_fromPrim($3 => JS_Boolean_toFFI_ToFFI_Bool_Boolean($3), $7 => JS_Boolean_fromFFI_FromFFI_Bool_Boolean($7), 'HTMLInputElement.getchecked', $c => $d => Web_Internal_HtmlPrim_HTMLInputElement_prim__checked($c, $d), $12 => $13 => $14 => Web_Internal_HtmlPrim_HTMLInputElement_prim__setChecked($12, $13, $14), $0);
}

Could you please print the output of pack info while in the root directory of the project (where bug.ipkg is located)? Also, could you please remove folder build and run make page again just to make sure it still produces the output you posted above (I've done that on my end, I always get what I posted in this comment).

gergoerdi commented 1 year ago
Package Collection  : nightly-230813
Idris2 URL          : https://github.com/idris-lang/Idris2
Idris2 Version      : 0.6.0
Idris2 Commit       : badf1e98c8812e8b54258754cf47782f61d8fb15
Scheme Executable   : chezscheme
Pack Commit         : 52131d2a562310055ea90832a84a74533042c259
Installed Libraries : algebra
                      ansi
                      base
                      bytestring
                      contrib
                      dom
                      dom-mvc
                      elab-util
                      filepath
                      finite
                      idris2
                      js
                      json-simple
                      linear
                      monocle
                      network
                      parser
                      parser-json
                      prelude
                      prim
                      quantifiers-extra
                      refined
                      rio
                      sop
                      tailrec
                      test
                      toml
gergoerdi commented 1 year ago

Removing /build and running make again doesn't change things.

stefan-hoeck commented 1 year ago

Could you please switch to the latest available pack collection and see if this helps: pack switch latest.

gergoerdi commented 1 year ago

That took a hot minute, but after everything got rebuilt, it's working now!

I'm not closing this ticket of course because this was originally about fixing checkbox updates.

stefan-hoeck commented 1 year ago

The issue originally described here is expected behavior. There is nothing to fix. However, we might add a checked command as a feature request, so yes, lets leave this open.