Bläddra i källkod

Refactor feedback view controller to be scroll view + refactor release name (#10975)

Tejas Deshpande 3 år sedan
förälder
incheckning
5788175ead

+ 20 - 56
FirebaseAppDistribution/Sources/FIRAppDistribution.m

@@ -55,6 +55,7 @@ NSString *const kDisplayVersionKey = @"displayVersion";
 NSString *const kAuthErrorMessage = @"Unable to authenticate the tester";
 NSString *const kAuthCancelledErrorMessage = @"Tester cancelled sign-in";
 NSString *const kFIRFADSignInStateKey = @"FIRFADSignInState";
+NSString *const kFIRFADReleaseNameKey = @"FIRFADReleaseName";
 
 @synthesize isTesterSignedIn = _isTesterSignedIn;
 
@@ -176,6 +177,21 @@ NSString *const kFIRFADSignInStateKey = @"FIRFADSignInState";
           return;
         }
 
+        // TODO: This will need to be updated everytime the app starts, if the user is signed in,
+        // as the release can change on updates, but the sign in state is peristent.
+        [FIRFADApiServiceSwift findReleaseWithDisplayVersion:[self getAppVersion]
+                                                buildVersion:[self getAppBuild]
+                                                    codeHash:[self getCodeHash]
+                                                  completion:^(NSString *__nullable releaseName,
+                                                               NSError *__nullable error) {
+                                                    if (error || releaseName == nil) {
+                                                      return;
+                                                    }
+                                                    [[GULUserDefaults standardUserDefaults]
+                                                        setObject:releaseName
+                                                           forKey:kFIRFADReleaseNameKey];
+                                                  }];
+
         [[GULUserDefaults standardUserDefaults] setBool:YES forKey:kFIRFADSignInStateKey];
         completion(nil);
       }];
@@ -350,73 +366,21 @@ NSString *const kFIRFADSignInStateKey = @"FIRFADSignInState";
 - (void)startFeedbackWithAdditionalFormText:(NSString *)additionalFormText
                                       image:(UIImage *_Nullable)image {
   if ([self isTesterSignedIn]) {
-    [self findReleaseAndStartFeedbackWithAdditionalFormText:additionalFormText image:image];
+    [self.uiService startFeedbackWithAdditionalFormText:additionalFormText image:image];
   } else {
     [self signInTesterWithCompletion:^(NSError *_Nullable error) {
       if (error) {
         return;
       }
-      [self findReleaseAndStartFeedbackWithAdditionalFormText:additionalFormText image:image];
+      [self.uiService startFeedbackWithAdditionalFormText:additionalFormText image:image];
     }];
   }
 }
 
-- (void)findReleaseAndStartFeedbackWithAdditionalFormText:(NSString *)additionalFormText
-                                                    image:(UIImage *_Nullable)image {
-  // TODO(tundeagboola) Because network requests can be slow, consider doing this check during app
-  // start
-  [self findReleaseWithCompletion:^(NSString *__nullable releaseName, NSError *__nullable error) {
-    if (error) {
-      //      TODO(tundeagboola) Show toast if 403 is returned which means tester doesn't have
-      //      access, or if a different network error occurs
-      return;
-    }
-    if (releaseName == nil) {
-      // TODO(tundeagboola) we need to handle this scenario even though releaseName should never be
-      // null if error is non-null
-      return;
-    }
-    [self.uiService startFeedbackWithAdditionalFormText:additionalFormText
-                                            releaseName:releaseName
-                                                  image:image];
-  }];
-}
-
 - (void)enableFeedbackOnScreenshotWithAdditionalFormText:(NSString *)additionalFormText
                                            showAlertInfo:(BOOL)showAlertInfo {
-  [self findReleaseWithCompletion:^(NSString *__nullable releaseName, NSError *__nullable error) {
-    if (error) {
-      //      TODO(tundeagboola) Show toast if 403 is returned which means tester doesn't have
-      //      access, or if a different network error occurs
-      return;
-    }
-    if (releaseName == nil) {
-      // TODO(tundeagboola) we need to handle this scenario even though releaseName should never be
-      // null if error is non-null
-      return;
-    }
-    [self.uiService enableFeedbackOnScreenshotWithAdditionalFormText:additionalFormText
-                                                         releaseName:releaseName
-                                                       showAlertInfo:showAlertInfo];
-  }];
-}
-
-- (void)findReleaseWithCompletion:(void (^)(NSString *_Nullable releaseName,
-                                            NSError *_Nullable error))findReleaseCompletion {
-  [FIRFADApiServiceSwift
-      findReleaseWithDisplayVersion:[self getAppVersion]
-                       buildVersion:[self getAppBuild]
-                           codeHash:[self getCodeHash]
-                         completion:^(NSString *__nullable releaseName, NSError *__nullable error) {
-                           if (error) {
-                             FIRFADErrorLog(@"Failed to identify release: Could not fetch %ld - %@",
-                                            [error code], [error localizedDescription]);
-                           }
-                           if (releaseName == nil) {
-                             FIRFADErrorLog(@"Failed to identify release: Release not returned");
-                           }
-                           findReleaseCompletion(releaseName, error);
-                         }];
+  [self.uiService enableFeedbackOnScreenshotWithAdditionalFormText:additionalFormText
+                                                     showAlertInfo:showAlertInfo];
 }
 
 #pragma mark - Swizzling disabled

+ 0 - 2
FirebaseAppDistribution/Sources/FIRAppDistributionUIService.h

@@ -57,11 +57,9 @@ typedef void (^AppDistributionRegistrationFlowCompletion)(NSError *_Nullable err
 - (void)showCheckForUpdatesUIAlertWithCompletion:(FIRFADUIActionCompletion)completion;
 
 - (void)startFeedbackWithAdditionalFormText:(NSString *)additionalFormText
-                                releaseName:(NSString *)releaseName
                                       image:(UIImage *_Nullable)image;
 
 - (void)enableFeedbackOnScreenshotWithAdditionalFormText:(NSString *)additionalFormText
-                                             releaseName:(NSString *)releaseName
                                            showAlertInfo:(BOOL)showAlertInfo;
 
 - (void)initializeUIState;

+ 23 - 13
FirebaseAppDistribution/Sources/FIRAppDistributionUIService.m

@@ -215,12 +215,10 @@ SFAuthenticationSession *_safariAuthenticationVC;
 // MARK: - In App Feedback
 
 - (void)startFeedbackWithAdditionalFormText:(NSString *)additionalFormText
-                                releaseName:(NSString *)releaseName
                                       image:(UIImage *__nullable)image {
   // TODO: Verify what happens when the string is empty.
   UIViewController *feedbackViewController =
       [FIRFADInAppFeedback feedbackViewControllerWithAdditionalFormText:additionalFormText
-                                                            releaseName:releaseName
                                                                   image:image
                                                               onDismiss:^() {
                                                                 // TODO: Consider using a
@@ -230,14 +228,17 @@ SFAuthenticationSession *_safariAuthenticationVC;
                                                                 // UIService to Swift.
                                                                 [self resetUIState];
                                                               }];
-  [self initializeUIState];
-  [self.hostingViewController presentViewController:feedbackViewController
-                                           animated:YES
-                                         completion:nil];
+
+  // TODO: Potentially move the check for the release version here.
+  if (feedbackViewController != nil) {
+    [self initializeUIState];
+    [self.hostingViewController presentViewController:feedbackViewController
+                                             animated:YES
+                                           completion:nil];
+  }
 }
 
 - (void)enableFeedbackOnScreenshotWithAdditionalFormText:(NSString *)additionalFormText
-                                             releaseName:(NSString *)releaseName
                                            showAlertInfo:(BOOL)showAlertInfo {
   // TODO: Consider adding showActionSheetBeforeFeedback parameter.
   if (!self.isListeningToScreenshot) {
@@ -247,7 +248,6 @@ SFAuthenticationSession *_safariAuthenticationVC;
                                                  name:UIApplicationUserDidTakeScreenshotNotification
                                                object:[UIApplication sharedApplication]];
     self.additionalFormText = additionalFormText;
-    self.releaseName = releaseName;
 
     NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
     BOOL dontShowAlert = [defaults boolForKey:kFIRFADScreenshotFeedbackUserDefault];
@@ -293,11 +293,21 @@ SFAuthenticationSession *_safariAuthenticationVC;
 - (void)screenshotDetected:(NSNotification *)notification {
   dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0ul), ^{
     [FIRFADInAppFeedback getManuallyCapturedScreenshotWithCompletion:^(UIImage *screenshot) {
-      dispatch_async(dispatch_get_main_queue(), ^{
-        [self startFeedbackWithAdditionalFormText:self.additionalFormText
-                                      releaseName:self.releaseName
-                                            image:screenshot];
-      });
+      if ([[FIRAppDistribution appDistribution] isTesterSignedIn]) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+          [self startFeedbackWithAdditionalFormText:self.additionalFormText image:screenshot];
+        });
+      } else {
+        [[FIRAppDistribution appDistribution]
+            signInTesterWithCompletion:^(NSError *_Nullable error) {
+              if (error) {
+                return;
+              }
+              dispatch_async(dispatch_get_main_queue(), ^{
+                [self startFeedbackWithAdditionalFormText:self.additionalFormText image:screenshot];
+              });
+            }];
+      }
     }];
   });
 }

+ 7 - 3
FirebaseAppDistribution/Tests/Unit/FIRAppDistributionTests.m

@@ -25,6 +25,8 @@
 #import "FirebaseCore/Extension/FirebaseCoreInternal.h"
 #import "FirebaseInstallations/Source/Library/Private/FirebaseInstallationsInternal.h"
 
+@import FirebaseAppDistributionInternal;
+
 @interface FIRAppDistributionTests : XCTestCase
 
 @property(nonatomic, strong) FIRAppDistribution *appDistribution;
@@ -47,6 +49,7 @@
 @implementation FIRAppDistributionTests {
   id _mockFIRAppClass;
   id _mockFIRFADApiService;
+  id _mockFIRFADApiServiceSwift;
   id _mockFIRAppDistributionUIService;
   id _mockFIRInstallations;
   id _mockInstallationToken;
@@ -67,6 +70,7 @@
   _mockBuildVersion = @"mock-build-version";
   _mockFIRAppClass = OCMClassMock([FIRApp class]);
   _mockFIRFADApiService = OCMClassMock([FIRFADApiService class]);
+  _mockFIRFADApiServiceSwift = OCMClassMock([FIRFADApiServiceSwift class]);
   _mockFIRAppDistributionUIService = OCMPartialMock([FIRAppDistributionUIService sharedInstance]);
   _mockFIRInstallations = OCMClassMock([FIRInstallations class]);
   _mockInstallationToken = OCMClassMock([FIRInstallationsAuthTokenResult class]);
@@ -478,7 +482,7 @@
   [self mockUIServiceShowUICompletion:YES];
   OCMStub([[self appDistribution] getAppVersion]).andReturn(@"different-version");
   OCMStub([[self appDistribution] getAppBuild]).andReturn(@"different-build");
-  OCMReject([_mockMachO codeHash]);
+  OCMStub([_mockMachO codeHash]).andReturn(@"this-is-old");
 
   XCTestExpectation *checkForUpdateExpectation =
       [self expectationWithDescription:@"Check for update does prompt user"];
@@ -501,7 +505,7 @@
   [self mockUIServiceShowUICompletion:YES];
   OCMStub([[self appDistribution] getAppVersion]).andReturn(@"different-version");
   OCMStub([[self appDistribution] getAppBuild]).andReturn(_mockBuildVersion);
-  OCMReject([_mockMachO codeHash]);
+  OCMStub([_mockMachO codeHash]).andReturn(@"this-is-old");
 
   XCTestExpectation *checkForUpdateExpectation =
       [self expectationWithDescription:@"Check for update does prompt user"];
@@ -524,7 +528,7 @@
   [self mockUIServiceShowUICompletion:YES];
   OCMStub([[self appDistribution] getAppVersion]).andReturn(_mockDisplayVersion);
   OCMStub([[self appDistribution] getAppBuild]).andReturn(@"different-build");
-  OCMReject([_mockMachO codeHash]);
+  OCMStub([_mockMachO codeHash]).andReturn(@"this-is-old");
 
   XCTestExpectation *checkForUpdateExpectation =
       [self expectationWithDescription:@"Check for update does prompt user"];

+ 1 - 0
FirebaseAppDistributionInternal.podspec

@@ -50,6 +50,7 @@ Pod::Spec.new do |s|
     s.dependency 'FirebaseCore', '~> 10.0'
     s.dependency 'FirebaseCoreExtension', '~> 10.0'
     s.dependency 'FirebaseInstallations', '~> 10.0'
+    s.dependency 'GoogleUtilities/UserDefaults', '~> 7.8'
 
     s.pod_target_xcconfig = {
       'GCC_C_LANGUAGE_STANDARD' => 'c99',

+ 24 - 55
FirebaseAppDistributionInternal/Resources/AppDistributionInternalStoryboard.storyboard

@@ -19,9 +19,6 @@
                         <subviews>
                             <navigationBar contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="Dia-oO-Nu0">
                                 <rect key="frame" x="0.0" y="47" width="390" height="44"/>
-                                <constraints>
-                                    <constraint firstAttribute="height" constant="44" id="ifE-ZP-DGh"/>
-                                </constraints>
                                 <items>
                                     <navigationItem title="Feedback" id="JNd-eF-IDH">
                                         <barButtonItem key="leftBarButtonItem" title="Back" id="OJS-4m-F97">
@@ -37,69 +34,43 @@
                                     </navigationItem>
                                 </items>
                             </navigationBar>
-                            <view contentMode="scaleToFill" misplaced="YES" translatesAutoresizingMaskIntoConstraints="NO" id="B8f-p9-FeY">
-                                <rect key="frame" x="0.0" y="92" width="390" height="84"/>
+                            <scrollView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" ambiguous="YES" translatesAutoresizingMaskIntoConstraints="NO" id="2yf-qw-KAC">
+                                <rect key="frame" x="0.0" y="91" width="390" height="753"/>
                                 <subviews>
-                                    <label opaque="NO" userInteractionEnabled="NO" contentMode="TopLeft" horizontalHuggingPriority="251" verticalHuggingPriority="251" lineBreakMode="tailTruncation" numberOfLines="0" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="dmh-tx-P50">
-                                        <rect key="frame" x="12" y="8" width="366" height="52"/>
+                                    <label opaque="NO" userInteractionEnabled="NO" contentMode="TopLeft" horizontalHuggingPriority="251" verticalHuggingPriority="251" fixedFrame="YES" lineBreakMode="tailTruncation" numberOfLines="0" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="dmh-tx-P50" customClass="AdditionalFormTextLabel" customModule="FirebaseAppDistributionInternal">
+                                        <rect key="frame" x="12" y="18" width="366" height="52"/>
+                                        <autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
                                         <string key="text">[This is placeholder text from the developer that is show to the tester before they provide feedback. It can contain links to more information.]</string>
                                         <fontDescription key="fontDescription" name=".AppleSystemUIFont" family=".AppleSystemUIFont" pointSize="13"/>
                                         <nil key="textColor"/>
                                         <nil key="highlightedColor"/>
                                     </label>
-                                </subviews>
-                                <color key="backgroundColor" red="0.94901960784313721" green="0.94901960784313721" blue="0.96862745098039216" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
-                                <constraints>
-                                    <constraint firstAttribute="bottom" secondItem="dmh-tx-P50" secondAttribute="bottom" constant="24" id="4bb-g4-c6l"/>
-                                    <constraint firstItem="dmh-tx-P50" firstAttribute="trailing" secondItem="B8f-p9-FeY" secondAttribute="trailing" constant="-12" id="4iq-ek-mx8"/>
-                                    <constraint firstItem="dmh-tx-P50" firstAttribute="top" secondItem="B8f-p9-FeY" secondAttribute="topMargin" id="bQc-lM-lBQ"/>
-                                    <constraint firstItem="dmh-tx-P50" firstAttribute="leading" secondItem="B8f-p9-FeY" secondAttribute="leading" constant="12" id="va6-fC-hjm"/>
-                                </constraints>
-                            </view>
-                            <imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="scaleAspectFit" horizontalHuggingPriority="251" verticalHuggingPriority="251" ambiguous="YES" translatesAutoresizingMaskIntoConstraints="NO" id="dsI-rN-5aL">
-                                <rect key="frame" x="16" y="302" width="358" height="442"/>
-                            </imageView>
-                            <view contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="5dc-R4-ELB">
-                                <rect key="frame" x="0.0" y="175" width="390" height="86"/>
-                                <subviews>
-                                    <textView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" misplaced="YES" text="Have feedback? Let us know how we can make this app better?." textAlignment="natural" translatesAutoresizingMaskIntoConstraints="NO" id="iNL-09-ZI6">
-                                        <rect key="frame" x="19" y="17" width="351" height="50"/>
+                                    <textView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" fixedFrame="YES" text="Placeholder text" textAlignment="natural" translatesAutoresizingMaskIntoConstraints="NO" id="iNL-09-ZI6">
+                                        <rect key="frame" x="12" y="98" width="351" height="50"/>
+                                        <autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
                                         <color key="backgroundColor" systemColor="systemBackgroundColor"/>
-                                        <color key="textColor" systemColor="labelColor"/>
-                                        <fontDescription key="fontDescription" type="system" pointSize="14"/>
+                                        <color key="textColor" white="0.66666666666666663" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/>
+                                        <fontDescription key="fontDescription" type="system" pointSize="18"/>
                                         <textInputTraits key="textInputTraits" autocapitalizationType="sentences"/>
                                     </textView>
+                                    <imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="scaleAspectFit" horizontalHuggingPriority="251" verticalHuggingPriority="251" fixedFrame="YES" translatesAutoresizingMaskIntoConstraints="NO" id="dsI-rN-5aL">
+                                        <rect key="frame" x="72" y="193" width="247" height="341"/>
+                                        <autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
+                                    </imageView>
                                 </subviews>
-                                <color key="backgroundColor" systemColor="systemBackgroundColor"/>
-                                <constraints>
-                                    <constraint firstItem="iNL-09-ZI6" firstAttribute="centerX" secondItem="5dc-R4-ELB" secondAttribute="centerX" id="6mW-Zs-1zw"/>
-                                    <constraint firstItem="iNL-09-ZI6" firstAttribute="leading" secondItem="5dc-R4-ELB" secondAttribute="leading" constant="16" id="GJF-wf-ZNr"/>
-                                    <constraint firstAttribute="bottom" secondItem="iNL-09-ZI6" secondAttribute="bottom" constant="16" id="GwE-1C-l0F"/>
-                                    <constraint firstItem="iNL-09-ZI6" firstAttribute="top" secondItem="5dc-R4-ELB" secondAttribute="top" constant="16" id="vDu-hu-JbG"/>
-                                    <constraint firstAttribute="trailing" secondItem="iNL-09-ZI6" secondAttribute="trailing" constant="16" id="ysz-oV-pEc"/>
-                                </constraints>
-                            </view>
+                                <viewLayoutGuide key="contentLayoutGuide" id="aub-fa-pXf"/>
+                                <viewLayoutGuide key="frameLayoutGuide" id="SIy-yQ-15v"/>
+                            </scrollView>
                         </subviews>
                         <viewLayoutGuide key="safeArea" id="6Tk-OE-BBY"/>
                         <color key="backgroundColor" systemColor="systemBackgroundColor"/>
                         <constraints>
-                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="top" secondItem="6Tk-OE-BBY" secondAttribute="top" id="7XR-Cu-c7J"/>
-                            <constraint firstItem="B8f-p9-FeY" firstAttribute="top" secondItem="8bC-Xf-vdC" secondAttribute="top" constant="91" id="EFg-iZ-Jzz"/>
-                            <constraint firstItem="5dc-R4-ELB" firstAttribute="trailing" secondItem="B8f-p9-FeY" secondAttribute="trailing" id="EwK-Jx-Hes"/>
-                            <constraint firstItem="6Tk-OE-BBY" firstAttribute="trailing" secondItem="dsI-rN-5aL" secondAttribute="trailing" constant="16" id="I52-Pj-PSq"/>
-                            <constraint firstAttribute="centerX" secondItem="6Tk-OE-BBY" secondAttribute="centerX" id="Jvv-bL-zyF"/>
-                            <constraint firstAttribute="bottom" secondItem="5dc-R4-ELB" secondAttribute="bottom" constant="583" id="OBu-rX-tfb"/>
-                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="centerX" secondItem="6Tk-OE-BBY" secondAttribute="centerX" id="UDB-uL-1PW"/>
-                            <constraint firstItem="5dc-R4-ELB" firstAttribute="centerX" secondItem="dsI-rN-5aL" secondAttribute="centerX" id="Z7g-ss-vlW"/>
-                            <constraint firstItem="B8f-p9-FeY" firstAttribute="trailing" secondItem="Dia-oO-Nu0" secondAttribute="trailing" id="b1e-eB-0Fc"/>
-                            <constraint firstAttribute="bottom" secondItem="B8f-p9-FeY" secondAttribute="bottom" constant="669" id="gbS-Iu-ikS"/>
-                            <constraint firstItem="5dc-R4-ELB" firstAttribute="leading" secondItem="B8f-p9-FeY" secondAttribute="leading" id="gwL-YB-JX5"/>
-                            <constraint firstItem="dsI-rN-5aL" firstAttribute="top" secondItem="5dc-R4-ELB" secondAttribute="bottom" constant="41" id="kP6-6f-o6b"/>
-                            <constraint firstItem="5dc-R4-ELB" firstAttribute="top" secondItem="Dia-oO-Nu0" secondAttribute="bottom" constant="84" id="ol5-x8-ptW"/>
-                            <constraint firstItem="dsI-rN-5aL" firstAttribute="leading" secondItem="6Tk-OE-BBY" secondAttribute="leading" constant="16" id="vfI-nc-TeW"/>
-                            <constraint firstItem="B8f-p9-FeY" firstAttribute="leading" secondItem="Dia-oO-Nu0" secondAttribute="leading" id="waP-fr-WQe"/>
-                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="leading" secondItem="6Tk-OE-BBY" secondAttribute="leading" id="wg8-jh-o4A"/>
-                            <constraint firstAttribute="centerY" secondItem="6Tk-OE-BBY" secondAttribute="centerY" id="zZh-sJ-7Gx"/>
+                            <constraint firstItem="2yf-qw-KAC" firstAttribute="top" secondItem="Dia-oO-Nu0" secondAttribute="bottom" id="44s-kl-Gy7"/>
+                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="centerX" secondItem="8bC-Xf-vdC" secondAttribute="centerX" id="9Tb-LV-gvu"/>
+                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="leading" secondItem="6Tk-OE-BBY" secondAttribute="leading" id="9fC-4a-fJl"/>
+                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="top" secondItem="6Tk-OE-BBY" secondAttribute="top" id="G04-yQ-2uN"/>
+                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="leading" secondItem="2yf-qw-KAC" secondAttribute="leading" id="SZY-Vo-mcA"/>
+                            <constraint firstItem="Dia-oO-Nu0" firstAttribute="trailing" secondItem="2yf-qw-KAC" secondAttribute="trailing" id="oib-fT-y3X"/>
                         </constraints>
                     </view>
                     <connections>
@@ -107,6 +78,7 @@
                         <outlet property="feedbackTextView" destination="iNL-09-ZI6" id="Ur9-nn-jm4"/>
                         <outlet property="navigationBar" destination="Dia-oO-Nu0" id="eVF-OS-PgH"/>
                         <outlet property="screenshotUIImageView" destination="dsI-rN-5aL" id="bRf-CH-E8Y"/>
+                        <outlet property="scrollView" destination="2yf-qw-KAC" id="h6g-aJ-OrA"/>
                     </connections>
                 </viewController>
                 <placeholder placeholderIdentifier="IBFirstResponder" id="dkx-z0-nzr" sceneMemberID="firstResponder"/>
@@ -115,9 +87,6 @@
         </scene>
     </scenes>
     <resources>
-        <systemColor name="labelColor">
-            <color red="0.0" green="0.0" blue="0.0" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
-        </systemColor>
         <systemColor name="systemBackgroundColor">
             <color white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/>
         </systemColor>

+ 1 - 0
FirebaseAppDistributionInternal/Sources/ApiService.swift

@@ -48,6 +48,7 @@ enum Strings {
   static let GoogleUploadFileName = "screenshot.png"
   static let contentTypeHeader = "Content-Type"
   static let jsonContentType = "application/json"
+  static let releaseNameKey = "FIRFADReleaseName"
 }
 
 enum AppDistributionApiError: NSInteger {

+ 240 - 12
FirebaseAppDistributionInternal/Sources/FeedbackViewController.swift

@@ -14,7 +14,7 @@
 
 import UIKit
 
-class FeedbackViewController: UIViewController {
+class FeedbackViewController: UIViewController, UITextViewDelegate {
   // TODO: Consider the situations where this instance is initiated once, and used
   // multiple times.
   var viewDidDisappearCallback: () -> Void = {}
@@ -27,27 +27,40 @@ class FeedbackViewController: UIViewController {
   @IBOutlet var additionalFormTextLabel: UILabel!
   @IBOutlet var feedbackTextView: UITextView!
   @IBOutlet var navigationBar: UINavigationBar!
+  @IBOutlet var scrollView: UIScrollView!
+
+  // MARK: - UIViewController
 
   override func viewDidLoad() {
     super.viewDidLoad()
     // Do any additional setup after loading the view.
 
+    feedbackTextView.isScrollEnabled = false
+    setScrollViewConstraints()
+    setAdditionalFormTextConstraints()
+    setFeedbackInputConstraints()
+    setScreenshotImageConstrains()
+
     let additionalFormText = additionalFormText
     if additionalFormText != nil {
       additionalFormTextLabel.text = additionalFormText
     }
 
-    let image = image
-    if image != nil {
+    feedbackTextView.delegate = self
+    resetFeedbackTextViewWithPlaceholderText()
+
+    if let image = image {
       screenshotUIImageView.image = image
+      self.image = nil
     }
   }
 
-  override func viewWillAppear(_ animated: Bool) {
-    screenshotUIImageView.image = image
-    image = nil
+  override func viewDidDisappear(_ animated: Bool) {
+    viewDidDisappearCallback()
   }
 
+  // MARK: - Actions
+
   @IBAction func tappedSend(_ sender: Any) {
     guard let releaseName = releaseName else {
       // TODO(tundeagboola) throw error or
@@ -82,6 +95,12 @@ class FeedbackViewController: UIViewController {
       }
   }
 
+  @IBAction func tappedCancel(_ sender: Any) {
+    dismiss(animated: true)
+  }
+
+  // MARK: - Utilties
+
   private func commitFeedback(feedbackName: String) {
     ApiService.commitFeedback(feedbackName: feedbackName) { error in
       if error != nil {
@@ -91,16 +110,225 @@ class FeedbackViewController: UIViewController {
     }
   }
 
-  @IBAction func tappedCancel(_ sender: Any) {
+  func feedbackSubmitted() {
+    // TODO(tundeagboola) show success toast
     dismiss(animated: true)
   }
 
-  override func viewDidDisappear(_ animated: Bool) {
-    viewDidDisappearCallback()
+  // MARK: - UI Constraints
+
+  func setScrollViewConstraints() {
+    scrollView.translatesAutoresizingMaskIntoConstraints = false
+    scrollView.alwaysBounceVertical = true
+    let bottomConstraint = NSLayoutConstraint(
+      item: scrollView!,
+      attribute: .bottom,
+      relatedBy: .equal,
+      toItem: view,
+      attribute: .bottom,
+      multiplier: 1,
+      constant: 0
+    )
+    let leftConstraint = NSLayoutConstraint(
+      item: scrollView!,
+      attribute: .left,
+      relatedBy: .equal,
+      toItem: navigationBar,
+      attribute: .left,
+      multiplier: 1,
+      constant: 0
+    )
+    let rightConstraint = NSLayoutConstraint(
+      item: scrollView!,
+      attribute: .right,
+      relatedBy: .equal,
+      toItem: navigationBar,
+      attribute: .right,
+      multiplier: 1,
+      constant: 0
+    )
+    view.addConstraints([bottomConstraint, leftConstraint, rightConstraint])
   }
 
-  func feedbackSubmitted() {
-    // TODO(tundeagboola) show success toast
-    dismiss(animated: true)
+  func setAdditionalFormTextConstraints() {
+    additionalFormTextLabel.translatesAutoresizingMaskIntoConstraints = false
+    additionalFormTextLabel.numberOfLines = 0
+
+    let topConstraint = NSLayoutConstraint(
+      item: additionalFormTextLabel!,
+      attribute: .top,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .top,
+      multiplier: 1,
+      constant: 0
+    )
+    let bottomConstraint = NSLayoutConstraint(
+      item: additionalFormTextLabel!,
+      attribute: .bottom,
+      relatedBy: .greaterThanOrEqual,
+      toItem: scrollView,
+      attribute: .top,
+      multiplier: 1,
+      constant: 40
+    )
+    let leftConstraint = NSLayoutConstraint(
+      item: additionalFormTextLabel!,
+      attribute: .left,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .left,
+      multiplier: 1,
+      constant: 0
+    )
+    let rightConstraint = NSLayoutConstraint(
+      item: additionalFormTextLabel!,
+      attribute: .right,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .right,
+      multiplier: 1,
+      constant: 0
+    )
+    scrollView.addConstraints([topConstraint, bottomConstraint, leftConstraint, rightConstraint])
+    let widthConstraint = NSLayoutConstraint(
+      item: additionalFormTextLabel!,
+      attribute: .width,
+      relatedBy: .equal,
+      toItem: navigationBar,
+      attribute: .width,
+      multiplier: 1,
+      constant: 0
+    )
+    view
+      .addConstraints([topConstraint, bottomConstraint, leftConstraint, rightConstraint,
+                       widthConstraint])
+
+    // TODO: Better color
+    additionalFormTextLabel.backgroundColor = .lightGray
+  }
+
+  func setFeedbackInputConstraints() {
+    feedbackTextView.translatesAutoresizingMaskIntoConstraints = false
+    let topConstraint = NSLayoutConstraint(
+      item: feedbackTextView!,
+      attribute: .top,
+      relatedBy: .equal,
+      toItem: additionalFormTextLabel,
+      attribute: .bottom,
+      multiplier: 1,
+      constant: 0
+    )
+    let bottomConstraint = NSLayoutConstraint(
+      item: feedbackTextView!,
+      attribute: .bottom,
+      relatedBy: .greaterThanOrEqual,
+      toItem: additionalFormTextLabel,
+      attribute: .top,
+      multiplier: 1,
+      constant: 100
+    )
+    let leftConstraint = NSLayoutConstraint(
+      item: feedbackTextView!,
+      attribute: .left,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .left,
+      multiplier: 1,
+      constant: 0
+    )
+    let rightConstraint = NSLayoutConstraint(
+      item: feedbackTextView!,
+      attribute: .right,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .right,
+      multiplier: 1,
+      constant: 0
+    )
+    let widthConstraint = NSLayoutConstraint(
+      item: feedbackTextView!,
+      attribute: .width,
+      relatedBy: .equal,
+      toItem: navigationBar,
+      attribute: .width,
+      multiplier: 1,
+      constant: 0
+    )
+    view
+      .addConstraints([topConstraint, bottomConstraint, leftConstraint, rightConstraint,
+                       widthConstraint])
+
+    feedbackTextView.textContainerInset = UIEdgeInsets(top: 20, left: 20, bottom: 20, right: 20)
+  }
+
+  func setScreenshotImageConstrains() {
+    screenshotUIImageView.translatesAutoresizingMaskIntoConstraints = false
+    let topConstraint = NSLayoutConstraint(
+      item: screenshotUIImageView!,
+      attribute: .top,
+      relatedBy: .equal,
+      toItem: feedbackTextView,
+      attribute: .bottom,
+      multiplier: 1,
+      constant: 0
+    )
+    let bottomConstraint = NSLayoutConstraint(
+      item: screenshotUIImageView!,
+      attribute: .bottom,
+      relatedBy: .greaterThanOrEqual,
+      toItem: scrollView,
+      attribute: .bottom,
+      multiplier: 1,
+      constant: 10
+    )
+    let leftConstraint = NSLayoutConstraint(
+      item: screenshotUIImageView!,
+      attribute: .left,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .left,
+      multiplier: 1,
+      constant: 0
+    )
+    let rightConstraint = NSLayoutConstraint(
+      item: screenshotUIImageView!,
+      attribute: .right,
+      relatedBy: .equal,
+      toItem: scrollView,
+      attribute: .right,
+      multiplier: 1,
+      constant: 0
+    )
+    let widthConstraint = NSLayoutConstraint(
+      item: screenshotUIImageView!,
+      attribute: .width,
+      relatedBy: .equal,
+      toItem: feedbackTextView,
+      attribute: .width,
+      multiplier: 1,
+      constant: 0
+    )
+    scrollView
+      .addConstraints([topConstraint, bottomConstraint, leftConstraint, rightConstraint,
+                       widthConstraint])
+  }
+
+  // MARK: - UITextViewDelegate
+
+  func textViewDidBeginEditing(_ textView: UITextView) {
+    textView.text = nil
+    textView.textColor = .black
+  }
+
+  func textViewDidEndEditing(_ textView: UITextView) {
+    if textView.text.isEmpty {
+      resetFeedbackTextViewWithPlaceholderText()
+    }
+  }
+
+  func resetFeedbackTextViewWithPlaceholderText() {
+    feedbackTextView.text = "Do you have any feedback?"
+    feedbackTextView.textColor = .lightGray
   }
 }

+ 16 - 6
FirebaseAppDistributionInternal/Sources/InAppFeedback.swift

@@ -16,15 +16,23 @@ import Foundation
 import UIKit
 import Photos
 
+// TODO: Fix spm to allow using this.
+// TODO: This is needed to send actual feedback.
+// import GoogleUtilities.GULUserDefaults
+
 @objc(FIRFADInAppFeedback) open class InAppFeedback: NSObject {
   @objc(
-    feedbackViewControllerWithAdditionalFormText:releaseName:image:onDismiss:
+    feedbackViewControllerWithAdditionalFormText:image:onDismiss:
   ) public static func feedbackViewController(additionalFormText: String,
-                                              releaseName: String,
                                               image: UIImage?,
                                               onDismiss: @escaping ()
                                                 -> Void)
-    -> UIViewController {
+    -> UIViewController? {
+    // TODO: Check for debug mode, and if it is, proceed even if it's not available, otherwise return nil
+    // Uncomment this to get access to the release name.
+//    let releaseName = GULUserDefaults.standard().string(forKey: Strings.releaseNameKey)
+    let releaseName = ""
+
     // TODO: Add the additionalInfoText parameter.
     let frameworkBundle = Bundle(for: self)
 
@@ -52,6 +60,8 @@ import Photos
   @objc(getManuallyCapturedScreenshotWithCompletion:)
   public static func getManuallyCapturedScreenshot(completion: @escaping (_ screenshot: UIImage?)
     -> Void) {
+    // TODO: There's a bug where the screenshot returned isn't the current screenshot
+    // but the previous screenshot.
     getPhotoPermissionIfNecessary(completionHandler: { authorized in
       guard authorized else {
         completion(nil)
@@ -71,11 +81,11 @@ import Photos
       requestOptions.isSynchronous = true
 
       let fetchResult = PHAsset.fetchAssets(with: .image, options: fetchOptions)
+      let asset = fetchResult.object(at: 0)
 
       manager.requestImage(
-        for: fetchResult.object(at: 0),
-        // TODO: Identify the correct size.
-        targetSize: CGSize(width: 358, height: 442),
+        for: asset,
+        targetSize: CGSize(width: asset.pixelWidth / 3, height: asset.pixelHeight / 3),
         contentMode: .aspectFill,
         options: requestOptions
       ) { image, err in

+ 44 - 0
FirebaseAppDistributionInternal/Sources/Internal/AdditionalFormTextLabel.swift

@@ -0,0 +1,44 @@
+// Copyright 2023 Google LLC
+//
+// 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 UIKit
+
+class AdditionalFormTextLabel: UILabel {
+  var topInset = 10.0
+  var bottomInset = 15.0
+  var leftInset = 10.0
+  var rightInset = 10.0
+
+  override func drawText(in rect: CGRect) {
+    let insets = UIEdgeInsets(
+      top: topInset,
+      left: leftInset,
+      bottom: bottomInset,
+      right: rightInset
+    )
+    super.drawText(in: rect.inset(by: insets))
+  }
+
+  override var intrinsicContentSize: CGSize {
+    let size = super.intrinsicContentSize
+    return CGSize(width: size.width + leftInset + rightInset,
+                  height: size.height + topInset + bottomInset)
+  }
+
+  override var bounds: CGRect {
+    didSet {
+      preferredMaxLayoutWidth = bounds.width - (leftInset + rightInset)
+    }
+  }
+}

+ 9 - 0
Package.swift

@@ -413,6 +413,7 @@ let package = Package(
         "FirebaseCore",
         "FirebaseCoreExtension",
         "FirebaseInstallations",
+        .product(name: "GULUserDefaults", package: "GoogleUtilities"),
       ],
       path: "FirebaseAppDistributionInternal/Sources"
     ),
@@ -434,6 +435,14 @@ let package = Package(
         .headerSearchPath("../../../.."),
       ]
     ),
+    .testTarget(
+      name: "AppDistributionInternalUnit",
+      dependencies: ["FirebaseAppDistributionInternal"],
+      path: "FirebaseAppDistributionInternal/Tests/Unit",
+      cSettings: [
+        .headerSearchPath("../../.."),
+      ]
+    ),
 
     .target(
       name: "FirebaseAuth",