Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Add binding onSet demo #37

Merged
merged 15 commits into from
Jul 4, 2024

Conversation

lambdaclan
Copy link
Contributor

Steps

  • Add your name or username and a link to your GitHub profile into the [Contributors.md][1] file.
  • Build the project on your machine. If it does not compile, fix the errors.
  • Describe the purpose and approach of your pull request below.
  • Submit the pull request. Thank you very much for your contribution!

Purpose

Relates to #33

Update the Demo application to include a demo of how to use the binding's onSet in order to react to changes to the binded variable.

Approach

The demo is using the EntryRow's text binding and is presented in the form of a rudimentary password validation form. The password is being checked while the user is typing in an async fashion where each validation check is dynamically updated with a visual indicator displaying its status. The validation statuses are buttons which on press display some information regarding the checks being performed as a popup dialog. The demo also shows how to correctly update from asynchronous contexts using Idle.

References

Screenshot from 2024-06-17 09-54-40

Screenshot from 2024-06-17 09-55-42

image

Demo

Screencast.from.2024-06-17.09-58-47.webm

Notes

  • I will be more than happy to do any modifications if required, so please let me know. Things like wording, demo page name etc.
  • I am fairly new to Swift, if the code can be improved in any way please let me know!
  • I also want to add some docs, but I could not find a way to install DocC on Linux. I have installed Swift and based on my research DocC should be included with the toolchain, but it is not there. For now, I might just add the text, and we go from there. Maybe we can use this example to explain onSet and how to update from asynchronous contexts? Let me know what you think.

Thank you for your time and consideration for this PR!

@david-swift
Copy link
Member

david-swift commented Jun 17, 2024

That's a really cool view! Thanks for writing it! Unfortunately, the implementation violates the "Single source of truth" principle by having two state variables kind of "for the same thing". What I mean by this is that the statuses can (and should) be calculated based on the password. Instead of using onSet, you can e.g. transform the state property into a computed property. I updated your example so that it follows the single source of truth design pattern, which drops onSet. Still, I do really love the example and would like to keep it, so we could rename it and add it using the single source of truth approach.

About DocC

For editing the tutorial, you can:

  • Clone https://github.com/apple/swift-docc and enter the directory
  • Build and install the executable named docc
  • Clone https://github.com/apple/swift-docc-render-artifact.git
  • Point the DOCC_HTML_DIR environment variable to the /dir folder of swift-docc-render-artifact
  • Enter the Sources/Adwaita/ directory of adwaita-swift.
  • Clone https://github.com/AparokshaUI/Adwaita.docc
  • Run docc preview

The preview does not work for the actual documentation (including articles) for some reasons, only for tutorials. As the onSet fits better into an article (it should really not be used that often), I'd simply use a Markdown editor as it is a Markdown file. I could then preview it on macOS and modify it. Note that the documentation has its own repo here as there is a lot of image material and code for the tutorial. An example article is available here, you can see that it really is basic Markdown.

@lambdaclan
Copy link
Contributor Author

Hello David, I hope you are doing well.

Apologies for the delayed reply but I got swamped with work 😅

That's a really cool view! Thanks for writing it!
...
Still, I do really love the example and would like to keep it
...

I am glad you decided to keep this example as I want to contribute to the project one way or another! I will be happy to do any necessary adjustments so that we can merge it.

Unfortunately, the implementation violates the "Single source of truth" principle by having two state variables kind of "for the same thing". What I mean by this is that the statuses can (and should) be calculated based on the password. Instead of using onSet, you can e.g. transform the state property into a computed property.

Thank you for the detailed explanation and for updating the code. This is exactly what I was hoping for when submitting this PR, an opportunity to learn something new and improve. I am generally aware of the "Single Source Of Truth" (SSOT) principle in software development, things like using API call data vs database cache data etc but after doing some research (https://developer.apple.com/videos/play/wwdc2020/10040/) it seems to also be particularly prevalent in SwiftUI.
Particularly, the statement below kind of cleared things out in my mind:

image

SSOT as a GUI paradigm is a new concept to me but If understood correctly the issue was although checkStatus was not directly monitoring the password value it was affecting its value whereas password state itself should have been the sole updater. So to summarize:

  • decide upon a single place to store a piece of data and have everywhere else read the same piece of data
  • never try to keep two values in sync or manually update one value when another one changes

It does make sense, but then I wonder as you also mention below:

onSet should really not be used that often

Under what circumstances should onSet be used over a computed property?

Furthermore, I was pleasantly surprised to see that through the use of computed properties async is not even needed any more which greatly simplifies the example. I guess this is because each checker keeps track of its own value through the computed property and just gets updated as it changes. This however raises another question.

If you remember, what started all this is that I wanted to react to a user typing into an EntryField but if the action is to actually just update some other relevant state (similar to the example here) the whole thing can just be wrapped as computed property and will work as expected. Async would have made more sense if I wanted to do other things in the background meaning non state value updates, things like downloading images etc is that correct?

Basically what I am asking is whether the statement that async is not necessary to update multiple values at the same time is correct or not.

About DocC
....

Thank you for explaining how to install DocC. I got it working and managed to load up the preview. It looks fantastic by the way!

As the onSet fits better into an article (it should really not be used that often), I'd simply use a Markdown editor as it is a Markdown file.

Understood, that would be no problem. I will be happy to create a new article once I have a deeper understanding of how things work hence my questions above 😇

In the meantime can you please let me know what changes are needed to proceed? Things like:

  • How should we name this example?
  • Is the modal approach OK for the demo, or would you prefer it if it was directly usable similar to the Counter demo?

In terms of:

The demo also shows how to correctly update from asynchronous contexts using Idle

I guess we will need a new demo for that, so I will try to think of something more suitable.

@david-swift
Copy link
Member

Under what circumstances should onSet be used over a computed property?

It can be used e.g. for debugging (detecting which view updates state), or when you have two state properties which are basically independent, but if one of them changes a certain way, you want to update the other one. As an example, take this line which removes the last element of an array if the user closes a dialog and another state value has a certain value, which it also updates. This might get quite complicated using a completely declarative approach with state values, so onSet can simplify this in certain cases.

Async would have made more sense if I wanted to do other things in the background meaning non state value updates, things like downloading images etc is that correct? Basically what I am asking is whether the statement that async is not necessary to update multiple values at the same time is correct or not.

Yes, async is not required for simple updates of values, it might be necessary if a state value depends on a process which is async in general (such as downloading images), that's why Idle is available.

I will be happy to create a new article once I have a deeper understanding of how things work hence my questions above 😇

Nice, thanks! 👍

How should we name this example?

I like the form section's title "Password Checker". I think we could replace "Binding Reactor" by "Password Checker".

Is the modal approach OK for the demo, or would you prefer it if it was directly usable similar to the Counter demo?

I think the modal approach is good.

I guess we will need a new demo for that, so I will try to think of something more suitable.

There is the Idle Demo with the progress bar example, but it might make sense to add an example for another common task such as networking (as the progress bar demo is an example for a repetitive task without really fetching any data or so).

@lambdaclan
Copy link
Contributor Author

Hello David, this should now be ready for a second review!

Please let me know if any more changes are needed.

Thank you!

Copy link
Member

@david-swift david-swift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@david-swift david-swift merged commit 9bcf802 into AparokshaUI:main Jul 4, 2024
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants