-
Notifications
You must be signed in to change notification settings - Fork 267
Conversation
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.
Solution for the current problem looks fine 🚢
I noticed though that I kind of find it weird that this is an "either or" decision for the observation modes in the first place … 😄
@@ -219,26 +219,15 @@ typedef NS_ENUM(NSInteger, BITFeedbackObservationMode) { | |||
This will grab the latest image from the camera roll. Requires iOS 7 or later! It also requires to add a NSPhotoLibraryUsageDescription to your app's Info.plist. | |||
- `BITFeedbackObservationModeThreeFingerTap`: Triggers when the user taps on the screen with three fingers. Takes a screenshot and attaches it to the composer. It also requires to add a NSPhotoLibraryUsageDescription to your app's Info.plist. | |||
|
|||
Default is `BITFeedbackObservationNone` | |||
Default is `BITFeedbackObservationNone`. | |||
If BITFeedbackManger was disabled, setting a new value will be ignored. |
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.
A test for this case might be nice 😊
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.
It is done ;)
- (void)setObservationModeThreeFingerTapEnabled:(BOOL)observationModeThreeFingerTapEnabled { | ||
_observationModeThreeFingerTapEnabled = observationModeThreeFingerTapEnabled; | ||
|
||
if(observationModeThreeFingerTapEnabled) { |
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.
Missing space
|
||
if (feedbackObservationMode == BITFeedbackObservationModeOnScreenshot) { | ||
[self setObservationModeOnScreenshotEnabled:YES]; | ||
if(self.observationModeThreeFingerTapEnabled) { |
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.
Missing space
@lumaxis I was thinking just the same. I was thinking of adding an |
/** | ||
* Allows both BITFeedbackObservationModeOnScreenshot and BITFeedbackObservationModeThreeFingerTap at the same time. | ||
*/ | ||
BITFeedbackObservationModeAll = 3 |
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.
I think this is not a problem in this specific case but generally I would advise for this to be some other, very high number, 99
or so for example. But since it's unlikely we'll add any more options here, this should be fine 🙂
Bug Description:
Disabling BITFeedbackManager didn't disable BITFeedbackObservationModes.