-
-
Notifications
You must be signed in to change notification settings - Fork 572
docs: corrected field state definitions #1540
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
Conversation
harry-whorlow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR! 🙌
Quick question - have you had a chance to check out PR #1456 and the related issues #1080 and [#1081]? I ask because those discussions were the main drivers behind the current layout and structure of the field state documentation. (We had a ton of interaction on those issues, so we wanted to emphasize the reasoning behind our decisions.)
The structure I originally went with was intentional and based on three layers:
General field state – covering value, validation, and errors.
Common interaction states – like isTouched, isDirty, and isPristine, which are most relevant for user interaction.
Introduction of isDefaultValue – added to reflect differences in how isDirty values are handled in React compared to Vue or Angular.
(Though I realize most of this was done in the React docs.)
Would you mind if I pushed a commit to your PR? There are a few things I’d like to tweak, and we can discuss whether they make sense. (I’ll also fix the isPristine type while I’m at it!) 😄
|
Sure, make whichever changes you like. I can fix up too, but probably not til Monday. The most important thing is that clicking the field is nothing to do with touched state. That's just simply wrong. |
|
whats your opinion on this? just updated the docs so all the libraries have the "Understanding 'isDirty' in Different Libraries" segment, and reorganised what you wrote a little. |
eda9802 to
45a13e6
Compare
|
There's a typo on the diagrams - isBlured missing an R I don't personally see the value in having 2 separate diagrams. They seem identical except for the last row, right? I'd use the "extended" one as the only one, with all 5 properties described above, then elaborate on dirty underneath. But not dying on that hill. At least it's accurate now. Not sure if it was intentional or not, but for Solid you've changed the syntax from |
|
@michaelboyles Ah, good catch on the typo! Yeah, I'm not particularly a fan of the double images, but given that the issues I linked were some of the more interacted-with ones—and people were asking for a clear explanation of our decisions—I don't mind using more space here to emphasise it. |
|
So this pr is looking good to go! Thanks for the submission, I'll get around to merging it in a bit. 🚀 |
Fixes #1169
"touched" definition in the docs say it's related to the user clicking the field. There isn't even a handler for clicking/focussing the field, so that's not right
The actual definition of touched is blurred/changed
Also added isBlurred and isDefaultValue states to the description, and further clarified that
isPristineis just the same as!isDirty