Skip to content

Conversation

@sweetmandm
Copy link

Quietly set the selection color and change the state

I'm using RSColorPicker in an app where the delegate method should only trip when the change is user-initiated.

I think this makes things a bit easier to manage because you can assume the selectionColor that comes through in the delegate method is a new value that needs to be handled.

This adds the following new methods. These old methods are still available, but default to calling quiet:YES. Also, -resizeOrRescale passes quiet:NO since the change was not user-initiated.

-setSelectionColor:quiet:
-handleStateChangedQuiet:
-handleStateChangedDisableActions:quiet:

P.S. sorry for the original non-descriptive PR name/comment, I misunderstood the syntax of my command line gh client

If 'quiet' is YES, do not call the colorPickerDidChangeSelection: delegate method.
@sweetmandm sweetmandm changed the title origin Quietly set the selection color and change the state Oct 8, 2014
@sweetmandm
Copy link
Author

NOTE: the same xctool command fails locally on 3484a7c593

xctool -project RSColorPicker.xcodeproj -scheme RSColorPicker -sdk iphonesimulator build test

With error message:

[Info] Collecting info for testables...2014-10-08 11:30:24.425 xctool[94257:907] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'launch path not accessible'

@RSully
Copy link
Owner

RSully commented Oct 8, 2014

That command is successful for me locally both in master and this branch. (Xcode 6.0.1, iOS 8.0, xctool 0.2.1)

PS: be sure to add your app to the list!

@RSully
Copy link
Owner

RSully commented Oct 8, 2014

I'm using RSColorPicker in an app where the delegate method should only trip when the change is user-initiated.

I think this makes things a bit easier to manage because you can assume the selectionColor that comes through in the delegate method is a new value that needs to be handled.

I'm curious about the use case here. Particularly, when is colorPickerDidChangeSelection: being called when it shouldn't be handled?

@sweetmandm
Copy link
Author

There are two cases I've observed:

  1. The delegate method is called if you manually set the selectionColor -- so if you want to load the picker with a preset value and you've already set its delegate, setting picker.selectionColor = newColor will cause handleStateChanged to be called in the selectionColor setter, which notifies the delegate.

  2. The delegate method will fire on resizeOrRescale because handleStateChanged is called there as well, even though the selectionColor has not changed.

Technically in the first case, the selection color did change, but since its change is part of the setup prior to user interaction, the delegate might not want to know about the change. In my case there is no way to distinguish whether colorPickerDidChangeSelection has come from the user or is a side effect of programmatically setting the selectionColor.

For my particular use-case it's important to know that a user has manually selected a color, because the concept of 'color' in my app is a combination of specified sRGB+Lab colors. If a user manually selects sRGB through the picker, an existing Lab portion is discarded. So it becomes important not to discard the Lab portion accidentally, even if the sRGB is identical.

@RSully
Copy link
Owner

RSully commented Oct 9, 2014

That makes sense. An alternative solution would be to add a second delegate method (say colorPickerDidChangeSelectionManually:) that is only called as a result of a touch event, while leaving the existing delegate method alone.

Or you could listen to the touchesBegan/touchesEnded delegate methods and discard there.

@sweetmandm
Copy link
Author

Yeah I thought about a userDidPickColor: delegate method but I was looking for minimal API impact, since other users might actually be expecting the delegate to fire all the time. I could also just set the delegate to nil then assign the new selection color and reset the delegate, so no worries if you think this isn't worth merging upstream, just trying to keep my deps nice and cocoa-podded :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants