Add delegate for implementing custom prompting behavior #65
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| // | ||
| // ARKEmailBugReportConfiguration.h | ||
| // Aardvark | ||
| // | ||
| // Created by Nick Entin on 4/14/18. | ||
| // Copyright 2018 Square, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
|
|
||
| @class ARKLogStore; | ||
| @class ARKEmailAttachment; | ||
|
|
||
|
|
||
| /// Configuration object describing the contents of an email bug report. | ||
| @interface ARKEmailBugReportConfiguration : NSObject | ||
|
|
||
| - (nonnull instancetype)init NS_UNAVAILABLE; | ||
| + (nonnull instancetype)new NS_UNAVAILABLE; | ||
|
|
||
| /// The email subject that will be prefilled when the email dialog is presented to the user. Defaults to an empty string. | ||
| @property (nonnull, nonatomic, copy) NSString *prefilledEmailSubject; | ||
|
|
||
| /// The log stores that will be included as attachments on the email. Defaults to an empty array. | ||
| @property (nonnull, nonatomic, copy) NSArray<ARKLogStore *> *logStores; | ||
|
|
||
| /// Controls whether or not a screenshot should be attached to the email, when available. Defaults to NO. | ||
| @property (nonatomic, readonly) BOOL includesScreenshot; | ||
|
|
||
| /// Controls whether or not a view hierarchy description should be attached to the email, when available. Defaults to NO. | ||
| @property (nonatomic, readonly) BOOL includesViewHierarchyDescription; | ||
|
|
||
| /// Additional attachments to include on the email. Defaults to nil. | ||
|
There was a problem hiding this comment. Might be a little more convenient to make it |
||
| @property (nullable, nonatomic, copy) NSArray<ARKEmailAttachment *> *additionalAttachments; | ||
|
|
||
| /// Removes the screenshot from the bug report, if one is included. | ||
| - (void)removeScreenshot; | ||
|
|
||
| /// Removes the view hierarchy description from the bug report, if one is included. | ||
| - (void)removeViewHierarchyDescription; | ||
|
There was a problem hiding this comment. It's a little confusing to use the word There was a problem hiding this comment. Would There was a problem hiding this comment. Actually, |
||
|
|
||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| // | ||
| // ARKEmailBugReportConfiguration.m | ||
| // Aardvark | ||
| // | ||
| // Created by Nick Entin on 4/14/18. | ||
| // Copyright 2018 Square, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // | ||
|
|
||
| #import "ARKEmailBugReportConfiguration.h" | ||
| #import "ARKEmailBugReportConfiguration_Protected.h" | ||
|
|
||
|
|
||
| @interface ARKEmailBugReportConfiguration () | ||
|
|
||
| @property (nonatomic, readwrite) BOOL includesScreenshot; | ||
| @property (nonatomic, readwrite) BOOL includesViewHierarchyDescription; | ||
|
|
||
| @end | ||
|
|
||
|
|
||
| @implementation ARKEmailBugReportConfiguration | ||
|
|
||
| - (instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; | ||
| { | ||
| self = [super init]; | ||
|
|
||
| _prefilledEmailSubject = @""; | ||
|
There was a problem hiding this comment. it may be worth having a quick comment here stating: "If any of the below default values change, remember to update the header file" That may also be overkill. I leave it up to you. There was a problem hiding this comment. I feel like making behavioral changes like changing default values should imply needing to update the header docs, and if it doesn't then you don't have enough header docs. Maybe that's just optimistic thinking, but adding comments saying that things are documented feels backwards. |
||
| _logStores = @[]; | ||
| _includesScreenshot = includesScreenshot; | ||
| _includesViewHierarchyDescription = includesViewHierarchyDescription; | ||
| _additionalAttachments = nil; | ||
|
|
||
| return self; | ||
| } | ||
|
|
||
| - (void)removeScreenshot; | ||
| { | ||
| self.includesScreenshot = NO; | ||
| } | ||
|
|
||
| - (void)removeViewHierarchyDescription; | ||
| { | ||
| self.includesViewHierarchyDescription = NO; | ||
| } | ||
|
|
||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // | ||
| // ARKEmailBugReportConfiguration_Protected.h | ||
| // Aardvark | ||
| // | ||
| // Created by Nick Entin on 5/14/18. | ||
| // Copyright 2018 Square, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // | ||
|
|
||
| @import Foundation; | ||
|
|
||
| #import "ARKEmailBugReportConfiguration.h" | ||
|
|
||
|
|
||
| @interface ARKEmailBugReportConfiguration (Protected) | ||
|
|
||
| - (nonnull instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; | ||
|
|
||
| @end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
|
|
||
|
|
||
| @class ARKEmailAttachment; | ||
| @class ARKEmailBugReportConfiguration; | ||
| @class ARKEmailBugReporter; | ||
| @class ARKLogStore; | ||
| @protocol ARKLogFormatter; | ||
|
|
@@ -53,6 +54,20 @@ | |
| @end | ||
|
|
||
|
|
||
| typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReportConfiguration *_Nullable configuration); | ||
|
|
||
| @protocol ARKEmailBugReporterPromptingDelegate <NSObject> | ||
|
There was a problem hiding this comment. The There was a problem hiding this comment. Looks like every other protocol in Aardvark does this, so just as well to match. |
||
|
|
||
| @required | ||
|
|
||
| /// Called on the main thread when a bug is filed to signal that a bug report prompt should be presented to the user. | ||
|
There was a problem hiding this comment. 👏 for calling out what thread this is called on |
||
| /// The `completion` should be called on the main thread with either an updated configuration to present the email dialog, or `nil` to signal that the prompt was cancelled. | ||
| /// When the initial `configuration` has `includesScreenshot` or `includesViewHierarchyDescription` false, setting the field to true will have no effect. | ||
|
There was a problem hiding this comment. "set to false"? But also: I'm not sure I get this comment. Why does changing some fields work, and some other fields not? Where should There was a problem hiding this comment. Basically it's a one-way door. If there is a screenshot, the initial configuration will have There was a problem hiding this comment. So then, why are these fields mutable at all? There was a problem hiding this comment. We still want the prompt to be able to remove the screenshot and view hierarchy. If we want to make the API more explicit here, we could do something like: /// Whether or not there is a screenshot attached to the bug report.
@property (nonatomic, readonly) BOOL includesScreenshot;
/// Removes the screenshot from the bug report if one is attached.
- (void)removeScreenshot;
/// Whether or not there is a view hierarchy description attached to the bug report.
@property (nonatomic, readonly) BOOL includesViewHierarchyDescription;
/// Removes the view hierarchy description from the bug report if one is attached.
- (void)removeViewHierarchyDescription;Alternatively, we could just attach the screenshot data, which would have the added advantage of allowing the custom prompt to modify the image. Although this would probably be dependent on #67 (and maybe #66). @property (nullable, nonatomic, copy) UIImage *screenshot;
@property (nullable, nonatomic, copy) NSString *viewHierarchyDescription;There was a problem hiding this comment. Talked about this offline with @efirestone and landed on going to |
||
| - (void)showBugReportingPromptForConfiguration:(ARKEmailBugReportConfiguration *_Nonnull)configuration completion:(ARKEmailBugReporterCustomPromptCompletionBlock _Nonnull)completion; | ||
|
|
||
| @end | ||
|
|
||
|
|
||
| /// Composes a bug report that is sent via email. | ||
| @interface ARKEmailBugReporter : NSObject <ARKBugReporter> | ||
|
|
||
|
|
@@ -73,6 +88,9 @@ | |
| /// The email attachment delegate, responsible for providing additional attachments and filtering which log stores to include in the bug report at the time the bug is filed. | ||
| @property (nullable, nonatomic, weak) id <ARKEmailBugReporterEmailAttachmentAdditionsDelegate> emailAttachmentAdditionsDelegate; | ||
|
|
||
| /// The prompting delegate, responsible for showing a prompt to file a bug report. When nil, an alert view will be shown prompting the user to input a title for the bug report. Defaults to nil. | ||
| @property (nullable, nonatomic, weak) id <ARKEmailBugReporterPromptingDelegate> promptingDelegate; | ||
|
|
||
| /// The formatter used to prepare the log for entry into an email. Defaults to a vanilla instance of ARKDefaultLogFormatter. | ||
| @property (nonnull, nonatomic) id <ARKLogFormatter> logFormatter; | ||
|
|
||
|
|
||
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.
triple-slash comment explaining what this object is?