Add binding onSet demo #37

Merged

lambdaclan merged 15 commits from binding-reactor-demo into main 2024-07-04 21:46:17 +02:00

Conversation 4 Commits 15 Files Changed 4 +206

lambdaclan commented 2024-06-17 03:59:00 +02:00 (Migrated from github.com)

Copy Link

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

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!

## Steps - [x] Add your name or username and a link to your GitHub profile into the [Contributors.md][1] file. - [x] Build the project on your machine. If it does not compile, fix the errors. - [x] Describe the purpose and approach of your pull request below. - [x] 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`](https://aparokshaui.github.io/adwaita-swift/documentation/adwaita/binding/onset(_:)) in order to react to changes to the binded variable. ## Approach The demo is using the EntryRow's [`text binding`](https://aparokshaui.github.io/adwaita-swift/documentation/adwaita/entryrow/init(_:text:)) 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`](https://aparokshaui.github.io/adwaita-swift/documentation/adwaita/idle). ## References    ## Demo [Screencast from 2024-06-17 09-58-47.webm](https://github.com/AparokshaUI/adwaita-swift/assets/47409392/c13fac13-008e-49fc-8eb3-fbccd7960641) ### 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 commented 2024-06-17 17:54:28 +02:00 (Migrated from github.com)

Copy Link

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.

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](https://github.com/AparokshaUI/adwaita-swift/pull/37/commits/1fc0647d9084e0d2cb4876d90b1b8effb0e915d1) 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](https://github.com/AparokshaUI/Adwaita.docc) as there is a lot of image material and code for the tutorial. An example article is available [here](https://github.com/AparokshaUI/Adwaita.docc/blob/main/Basics/KeyboardShortcuts.md), you can see that it really is basic Markdown.

lambdaclan commented 2024-06-25 05:44:23 +02:00 (Migrated from github.com)

Copy Link

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:

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.

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:  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](https://aparokshaui.github.io/adwaita-swift/documentation/adwaita/idle) I guess we will need a new demo for that, so I will try to think of something more suitable.

david-swift commented 2024-06-26 11:32:15 +02:00 (Migrated from github.com)

Copy Link

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).

> 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](https://github.com/david-swift/Memorize/blob/d0a52fb4acf07c5844ee54e5085a1639a0e76e5b/Sources/View/SetOverview.swift#L74) 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! :+1: > 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).

👍 1

lambdaclan commented 2024-07-04 05:04:36 +02:00 (Migrated from github.com)

Copy Link

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

Please let me know if any more changes are needed.

Thank you!

Hello David, this should now be ready for a second review! Please let me know if any more changes are needed. Thank you!

david-swift (Migrated from github.com) approved these changes 2024-07-04 21:46:08 +02:00

david-swift (Migrated from github.com) left a comment

Copy Link

Thank you!

Thank you!

Sign in to join this conversation.

Reviewers

No reviewers

david-swift

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

wontfix

This won't be fixed

No Label

bug

duplicate

enhancement

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No Milestone

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: aparoksha/adwaita-swift#37

No description provided.