Skip to content
This repository has been archived by the owner on Feb 19, 2020. It is now read-only.

Fix/fix disabling feedback #390

Merged
merged 5 commits into from
Jan 12, 2017
Merged

Conversation

ElektrojungeAtWork
Copy link
Contributor

Bug Description:
Disabling BITFeedbackManager didn't disable BITFeedbackObservationModes.

Copy link
Member

@lumaxis lumaxis left a 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.
Copy link
Member

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 😊

Copy link
Contributor Author

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) {
Copy link
Member

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) {
Copy link
Member

Choose a reason for hiding this comment

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

Missing space

@ElektrojungeAtWork
Copy link
Contributor Author

@lumaxis I was thinking just the same. I was thinking of adding an ObservationModeAll that allows everything active. I might do that before merging this PR ;)
Thx for the review!

/**
* Allows both BITFeedbackObservationModeOnScreenshot and BITFeedbackObservationModeThreeFingerTap at the same time.
*/
BITFeedbackObservationModeAll = 3
Copy link
Member

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 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants