SFBdragon
commented
2 months ago Just noting this quickly as it came up in the notification:
Percy only updates the element if it has changed.
This isn't the case here. We don't want to let the checkbox value change if the user clicks on it but app state didn't change. So .checked is always set when applying patch, regardless of whether the value was updated or not.
-------- Original Message -------- On 2024/09/24 18:49, Chinedu Francis Nwafili wrote:
@chinedufn commented on this pull request.
In crates/percy-dom/src/patch/apply_patches.rs:
@@ -241,7 +241,13 @@ fn apply_element_patch( } } AttributeValue::Bool(val_bool) => {
- if *val_bool {
- // Use
set_checkedinstead of{set,remove}_attributefor thecheckedattribute.If we reverted the behavior, there would be no way to specify the default checkedness besides resetting it after every percy update.
Percy only updates the element if it has changed.
So, if your element always looks like this:
html
!
{
<input
type
=
"checked"
checked=
true
/>
}
then it will always be checked.
So, there is a way to specify the default checkedness. Just set the attribute and never change it. percy will never touch it again if the VirtualNode never changes.
Not saying that relying on this is intuitive. Just mentioning so that we're designing on top of truthful ground.
The application has a map that holds option-value pairs, it starts off empty. The GUI has a form that holds a bunch of options (with default states)
If you are running client-side code you should be expected to render declaratively.
Your state should mimic what you see.
If you want to deviate from that model then don't create the element using percy.
Just create it yourself and append it to your percy element.
fn
render
(
)
->
VirtualNode
{
html
!
{
<div id=
'
container-
for
-non-percy-elements
'
}
}
fn
attach_form
(
)
{
let
form = document
.
create_element
(
"form"
)
;
let
input = document
.
create_element
(
"input"
)
;
// ...
let
container = document
.
get_element
(
"container-for-non-percy-elements"
)
;
container
.
append_child
(
form
)
;
}
What I'm proposing above is:
-
if you are running percy client side, we expect that you are using percy's html! macro / VirtualNode::* APIs declaratively.
-
As in, whatever your html! says is what the user will see
if you want to deviate from that model, do some outside of percy. Create the elements yourself, then append them yourself to a percy element.
-
So, say you're at a company that has been using a jQuery form library for 20 years. Cool, no problem. Just call a function to append that to your percy element
-
percy won't touch those elements. it doesn't know about them. it only knows about it's VirtualNodes
-
unless of course you delete the
that you created using percy -
so, in short, you can embed whatever you want inside of your percy-managed elements. If you need to do weird stuff, embed the elements yourself.
-
then it just becomes our job to provide examples and guides that arm people with the knowledge of when and how to leave percy to do edge case things. Using regular old HTML primitives.
-
because people will generally think that they have to use the library for everything. You don't. You can side-step it whenever you want. So, for extreme edge cases like this, we shouldn't make percy account for it. The user can just side-step percy.
A good default for CSR (follow the application state, lock the GUI state to what is explicitly specified using html!) isn't a good default for SSR (let the browser do its thing, don't try to lock the GUI state).
I addressed here why this isn't true for complex applications:
But, in short, for complex applications SSR rendering must also be based on application state (otherwise you're maintaining two very different rendering environments, double the work), its just that the GUI is an HTML string. But we want that GUI (HTML string) to be rendered in the same way that we render to the DOM. Anything that makes CSR and SSR different should be very well justified.
Let's assume that a developer might want to ensure the checkbox has a specific default checkedness (again, this is exclusively considering CSR, not SSR).
Create the element outside of percy and then append it to your percy-managed DOM tree.
You get exactly what you want, and percy doesn't need a complicated API for a vanishingly small edge case.
Another thing to note about the philosophy behind percy: we aren't trying to hide the DOM for you.
The fact that we give you a virtual dom is because we don't think there's a better way to manage tremendous amounts of application state.
We'd much prefer if instead of learning about virtual doms you could just learn HTML/real-DOM stuff. So, when real-DOM stuff handles the job well we can just point users in that direction. If the use cases is common we can abstract it for convenience.
This is why I appreciate the technical details in your docs. We don't want people to have to learn percy. We want them to learn HTML/DOM. Much more useful knowledge.
can we assume that SSR vnodes will only be created, not diff+patch
yes.
We just need to ensure that checked sets the HTML attribute when vnode.to_string()
Making SSR and CSR different is undesirable for reasons mentioned above. Unless there's ever a compelling reason.
Solutions proposals:
Option 4 - percy doesn't handle default checkedness at all and if you want on the client-side the book recommends just creating the DOM element yourself using web-sys. We some day also create an example of this in the examples/ directory.
This is better for the 99%+ of users that will never care about any of this.
And better for the <1% that do since instead of teaching them a weird percy-specific attribute we can instead teach them that "Hey .. by the way you can just jump outside percy and do real DOM stuff if you like ... here's how to do it for default checknedess". They are now armed with knowledge that will be useful for all future edge cases that they run into. "If percy isn't designed for my strange use case, I can just write my own code to handle it!"
vs. "reading through docs learning about 20 percy-specific concepts that didn't need to exist."
I operate under the assumption that our users would prefer to do things themselves correctly in a knowledge-transferable way than to have us give them a weird abstraction. They just need us to point them at how to do it.
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>
chinedufn
This PR changes the meaning of
checkedas specified in thehtml!macro.Originally, specifying
checkedwould add or remove thecheckedHTML attribute. This changed the default checkedness of the input element, but may or may not actually change the rendered state of the element.Now, specifying
checkeddirectly specifies the rendered state of the element. (The default checkedness is not modified, and can be set manually.)Motivation
This change makes it easier to do the more commonly-desired task of specifying the rendered state of the checkbox (likely according to application state) rather than merely the default checkedness.
This change mitigates shooting ourselves in the foot by assuming
checkedsets a rendered checkbox's checkedness.Further discussion is available in the issue #205 as well as in the new book entry and tests
https://github.com/SFBdragon/percy/tree/checked_sets_checkedness/book/src/html-macro/special-attributes
https://github.com/SFBdragon/percy/blob/checked_sets_checkedness/crates/percy-dom/tests/checked_attribute.rs
This resolves #205 for the time being.