-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Bind to properties instead of attributes by default #2022
Conversation
From #1322
I think this is something to add to the TODO, the ability to set types other than string to properties. |
So I actually think we can limit the scope of this PR to just swapping over from setting attributes to properties, this should allow us to rip out all the special treatment of |
If that's the case, then I guess most of the functionality is already implemented. The one thing unresolved for me is what to do when the property is removed, at the moment you are trying to delete the property which doesn't work and I'm not sure it would be correct even if it did. Did you test it? I didn't get to manually testing the behavior so if you could provide a test case for it, that would be nice. I don't see why it wouldn't work
Leaving it as-is is a bad idea. There are cases where the value must be removed.
I think this is the default so we can do that. |
Pretty much, though the special treatment of
Yep, and tested it in the browser without Rust and you cannot delete a HTMLElement property. I could add a test case but I don't think removing the property is the correct thing to do regardless of whether it worked or not.
In what cases? And when you do - are you expecting the state of the property to reflect what it was on the initial render or just |
I can do it here. I'm not sure which code exactly needs to be pulled out.
I was thinking of custom elements when I said that. I don't know what the default value should be |
I just don't know of a scenario where you want to delete an element property, which is why I'm asking :) |
f380699
to
3b31202
Compare
hours wasted: *many*
@mc1098, after ripping out the special treatment, 3 of the tests fail. The const inputElement = document.createElement('input')
inputElement.hasOwnProperty('value') // this returns false The value is set as attribute, not property. Seems like we might need the special treatment after all |
These tests are about `value` and `checked`. These attributes got special treatment before and therefore had tests which performed `PartialEq` operation to make sure those attributes are set and compared properly. Now, the special treatment is gone so the test fails because the value aren't set/compared in Rust
# Conflicts: # packages/yew/src/virtual_dom/mod.rs # packages/yew/src/virtual_dom/vtag.rs
There's too much changes happening to justify this PR. It is something for the future |
Description
Fixes #1322
TODOs
Checklist
cargo make pr-flow