Material: Switch calls valueChanged even when set programatically

[sovata8]

I am having some troubled with a Switch.
I need to set its state, without any side-effects (such as a call to valueChanged)

I am setting it programmatically, using the setOn (which from what I can see calls setSwitchState). In setSwitchState I see that it calls the action for event 'valueChanged'. Is this how it is supposed to be? I thought that these UIControl calls should only happen for user-interaction-initiated events, not when doing things programmatically. How do I change the switch without triggering valueChanged?

[danieldahan]

Can you show me where this is stated and I can make an update. Also, if you set the value programmatically, it doesn't necessarily mean you do not want to know in your delegation handle. Are you doing this on initialization?

[sovata8]

iOS/Switch.swift (78163a3, latest), lines:
352 and 359 (sendActions)
354 and 361 (delegate.switchDidChangeState, which I also think should not be called)

I am configuring on initialisation but also later on (when I update a Switch in one part of the app, another one needs to update elsewhere).

I think these should be called iff the state changes as a result of user interaction, but should not be called when the state changes programmatically. This is so that we can update the 'view' without triggering any side-effects. And if we want to trigger those side effects, we can always make the calls programmatically ourselves. This is also consistent with how UIKit's UISwitch works (I just tested it and it works as expected, no valueChanged calls when setting it programmatically). It's also how all other controls work in UIKit as far as I know. Even the documentation states

"UIKit > UIControlEvents > valueChanged: A touch dragging or otherwise manipulating a control, causing it to emit a series of different values."

and even more to the point:

UIKit > UISwitch > setOn(_:animated:)
Discussion
Setting the switch to either position does not result in an action message being sent.

[sovata8]

Upon closer look, setSwitchState should sometimes call the valueChanged and delegate methods, but only when called from a UI interaction context (e.g. in the current code: handleTouchUpOutsideOrCanceled, maybe there is more).

I guess a solution would be to overload setSwitchState with a private/internal version that along with the current arguments, takes a Bool for isCausedByUIInteraction. The current public setSwitchState would keep its signature and all it would do is call the new setSwitchState, passing it isCausedByUIInteraction = false. But for example the handleTouchUpOutsideOrCanceled function will call setSwitchState, passing it isCausedByUIInteraction = true. Then this new Bool will determine whether to call the valueChanged and delegate methods.

Of course this would cause issues for people who are relying on these calls when setting Switch's state, they would have to make the calls themselves.

[danieldahan]

I am going to come back to this issue later today. Sorry for any delays :)

[danieldahan]

So I like the reasoning and there is documentation that reinforces your notion. So let's make it happen. I will make an update to Material with your suggested behaviour. Thank you very much for bringing this to my attention. I will notify you when I make an update later today or tonight depending on some other issues I am working on.

[sovata8]

That's great! 👍 Thanks for the super quick response and for taking this suggestion on board :)

[danieldahan]

Can you test the development branch and let me know if the behaviour is correct? If so, then I can make a release.

[sovata8]

Hi, sorry for the late reply, was away from keyboard during the weekend.
I just tested the dev branch's version of Switch and it all works as expected (and consistent with UIKit's UIControl's behaviours). Nice work!

I did a search through the source code for more potential cases of the 'non-recommended' UIControl callbacks (that are now fixed or Switch). I found some such cases but I am not sure if I should continue writing under this thread or open a new issue (or perhaps contact you personally in some way?). I also have a suggestion for the implementation of the solution that I see in Switch (to avoid global variables and state). Not sure what's the most appropriate place to discuss these things. Let me know.

[danieldahan]

Please share here :) Also, I don't know of any global variables in the Switch. Another place to discuss, is at the Material Gitter channel, which is great as well, and probably better in this case.

[sovata8]

So, expanding on the new implementation for Switch, here are some other areas where it would be good to implement that new approach:

I did a search in the Material source code for some other places that are currently potentially triggerring delegates even when changed programatically (i.e. not via a user interaction).

I found two such places:

1. TabBar.swift
   open func animate(to button: UIButton, completion: ((UIButton) -> Void)? = nil) {
   always calls:
   s.delegate?.tabBar?(tabBar: s, didSelect: button)
   but should probably only call the delegate it when triggered from:
   internal func handleButton(button: UIButton) {
   and for example should NOT call the delegate when triggered from:
   open func select(at index: Int, completion: ((UIButton) -> Void)? = nil) {

2. NavigationDrawerController.swift
   open func openLeftView(velocity: CGFloat = 0) {
   and the related:
   openRightView, closeLeftView, closeRightView
   always call:
   delegate?.navigationDrawerController?(navigationDrawerController: self, ...
   but should probably only call the delegate it when triggered from:
   fileprivate func handleLeftViewPanGesture(recognizer: UIPanGestureRecognizer) {
   and the related:
   handleLeftViewTapGesture, handleRightViewPanGesture, handleRightViewTapGesture

So the suggestion would be to replicate the 'fix' that was done for Switch to these other places.

[danieldahan]

Yes, I agree. Also, I like your suggestion earlier to not use a variable to maintain the interaction state, but to use a function with encapsulated logic :) Great job. This should be done tonight or tomorrow, where I will ask for you to test again. Thank you!

[danieldahan]

Updated development with your suggestions :) I will finish the other controls tomorrow with a release.

[danieldahan]

So this is now in Material 2.4.1. I applied the fix for both the Switch and TabBar, but not the NavigationDrawerController. That is not a control as the others are. That is more of an architectural controller and the delegation methods are meant to hook features with or without user interaction. Not like the Switch which is a direct control. We can discuss this more if you like, but I didn't feel it was right, so I didn't make the change. I am open to a discussion as always :) Thanks again buddy!

[danieldahan]

@sovata8 I am going to make the same changes to the NavigationDrawer and new components coming out. I have been thinking this over and I think that all delegation methods should only fire on user initiated interactions. :) Thanks again

This will be out in Material 2.5.0

[sovata8]

Hi. Sorry for the late reply, holidays etc.

Glad to hear you're implementing that approach! Good luck and thanks again for creating that amazing project and being so open to discussions and suggestions :)