Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(feedback): accessibility audit action items #4698

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 0 additions & 1 deletion Samples/iOS-ObjectiveC/iOS-ObjectiveC/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ - (BOOL)application:(UIApplication *)application
config.configureWidget = ^(SentryUserFeedbackWidgetConfiguration *_Nonnull widget) {
if ([args containsObject:@"--io.sentry.feedback.auto-inject-widget"]) {
widget.labelText = @"Report Jank";
widget.widgetAccessibilityLabel = @"io.sentry.iOS-Swift.button.report-jank";
widget.layoutUIOffset = layoutOffset;
} else {
widget.autoInject = NO;
Expand Down
20 changes: 10 additions & 10 deletions Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
func testUIElementsWithDefaults() {
launchApp(args: ["--io.sentry.feedback.all-defaults"])
// widget button text
XCTAssert(app.staticTexts["Report a Bug"].exists)
XCTAssert(app.otherElements["Report a Bug"].exists)

widgetButton.tap()

Expand All @@ -42,11 +42,11 @@
XCTAssert(app.staticTexts["Report a Bug"].exists)

// form buttons
XCTAssert(app.staticTexts["Add a screenshot"].exists)

Check failure on line 45 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testUIElementsWithDefaults, XCTAssertTrue failed
XCTAssert(app.staticTexts["Cancel"].exists)
XCTAssert(app.staticTexts["Send Bug Report"].exists)

addScreenshotButton.tap()

Check failure on line 49 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testUIElementsWithDefaults, Failed to tap "io.sentry.feedback.form.add-screenshot" Button: No matches found for Elements matching predicate '"io.sentry.feedback.form.add-screenshot" IN identifiers' from input {(
XCTAssert(app.staticTexts["Remove screenshot"].exists)

// Input field placeholders
Expand All @@ -63,10 +63,10 @@
}

func testUIElementsWithCustomizations() {
launchApp()
launchApp(args: ["--io.sentry.feedback.auto-inject-widget"])

// widget button text
XCTAssert(app.staticTexts["Report Jank"].exists)
XCTAssert(app.otherElements["Report Jank"].exists)

widgetButton.tap()

Expand All @@ -75,10 +75,10 @@

// form buttons
XCTAssert(app.staticTexts["Report that jank"].exists)
XCTAssert(app.staticTexts["Show us the jank"].exists)

Check failure on line 78 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testUIElementsWithCustomizations, XCTAssertTrue failed
XCTAssert(app.staticTexts["What, me worry?"].exists)

addScreenshotButton.tap()

Check failure on line 81 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testUIElementsWithCustomizations, Failed to tap "io.sentry.feedback.form.add-screenshot" Button: No matches found for Elements matching predicate '"io.sentry.feedback.form.add-screenshot" IN identifiers' from input {(
XCTAssert(app.staticTexts["Oof too nsfl"].exists)

// Input field placeholders
Expand Down Expand Up @@ -327,10 +327,10 @@

messageTextView.tap()
messageTextView.typeText("UITest user feedback")
// first swipe down dismisses the keyboard that's still visible from typing the above inputs

// dismiss the onscreen keyboard
app.swipeDown(velocity: .fast)

// the modal cancel gesture
app.swipeDown(velocity: .fast)

Expand All @@ -353,7 +353,7 @@
func testAddingAndRemovingScreenshots() {
launchApp(args: ["--io.sentry.feedback.all-defaults"])
widgetButton.tap()
addScreenshotButton.tap()

Check failure on line 356 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testAddingAndRemovingScreenshots, Failed to tap "io.sentry.feedback.form.add-screenshot" Button: No matches found for Elements matching predicate '"io.sentry.feedback.form.add-screenshot" IN identifiers' from input {(
XCTAssert(removeScreenshotButton.isHittable)
XCTAssertFalse(addScreenshotButton.isHittable)
removeScreenshotButton.tap()
Expand All @@ -378,7 +378,7 @@
XCTAssertEqual(try getMarkerFileContents(type: .onSubmitError), "io.sentry.error;1;The user did not complete the feedback form.;description")

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information. Please check the following field: description."].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following field: description."].exists)

app.buttons["OK"].tap()
}
Expand All @@ -402,7 +402,7 @@
XCTAssertEqual(try getMarkerFileContents(type: .onSubmitError), "io.sentry.error;1;The user did not complete the feedback form.;thine email;thy complaint")

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information. Please check the following fields: thine email and thy complaint."].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following fields: thine email and thy complaint."].exists)

app.buttons["OK"].tap()
}
Expand All @@ -429,7 +429,7 @@
XCTAssertEqual(try getMarkerFileContents(type: .onSubmitError), "io.sentry.error;1;The user did not complete the feedback form.;thine email;thy complaint;thy name")

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information. Please check the following fields: thy name, thine email and thy complaint."].exists)
XCTAssert(app.staticTexts.element(matching: NSPredicate(format: "label LIKE 'You must provide all required information before submitting. Please check the following fields: thy name, thine email and thy complaint.'")).exists)

app.buttons["OK"].tap()
}
Expand All @@ -448,7 +448,7 @@
XCTAssertEqual(try getMarkerFileContents(type: .onSubmitError), "io.sentry.error;1;The user did not complete the feedback form.;description")

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information. Please check the following field: description."].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following field: description."].exists)

app.buttons["OK"].tap()
}
Expand Down Expand Up @@ -541,9 +541,9 @@
func assertFormHookFile(type: HookMarkerFile, exists: Bool) throws {
let path = try path(for: type)
if exists {
XCTAssert(fm.fileExists(atPath: path), "Expected file to exist at \(path)")

Check failure on line 544 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testSubmissionErrorThenSuccessAfterFixingIssues, XCTAssertTrue failed - Expected file to exist at /Users/runner/Library/Developer/CoreSimulator/Devices/4048116C-AFD0-42B0-A5CE-0627C1D15AFD/data/Containers/Data/Application/56A151ED-B9EA-407F-B6CF-CD98B7138512/Library/Application Support/io.sentry/feedback/onFormClose

Check failure on line 544 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testSubmitWithOnlyRequiredFieldsFilled, XCTAssertTrue failed - Expected file to exist at /Users/runner/Library/Developer/CoreSimulator/Devices/4048116C-AFD0-42B0-A5CE-0627C1D15AFD/data/Containers/Data/Application/56A151ED-B9EA-407F-B6CF-CD98B7138512/Library/Application Support/io.sentry/feedback/onFormClose
} else {
XCTAssertFalse(fm.fileExists(atPath: path), "Expected file to not exist at \(path)")

Check failure on line 546 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testSubmissionErrorThenSuccessAfterFixingIssues, XCTAssertFalse failed - Expected file to not exist at /Users/runner/Library/Developer/CoreSimulator/Devices/4048116C-AFD0-42B0-A5CE-0627C1D15AFD/data/Containers/Data/Application/56A151ED-B9EA-407F-B6CF-CD98B7138512/Library/Application Support/io.sentry/feedback/onFormOpen

Check failure on line 546 in Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

testSubmitWithOnlyRequiredFieldsFilled, XCTAssertFalse failed - Expected file to not exist at /Users/runner/Library/Developer/CoreSimulator/Devices/4048116C-AFD0-42B0-A5CE-0627C1D15AFD/data/Containers/Data/Application/56A151ED-B9EA-407F-B6CF-CD98B7138512/Library/Application Support/io.sentry/feedback/onFormOpen
}
}

Expand Down
4 changes: 4 additions & 0 deletions Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,7 @@
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.iOS-SwiftUITests.xctrunner";
"PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*]" = "";
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "SENTRY_UI_TEST $(inherited)";
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
TEST_TARGET_NAME = "iOS-Swift";
Expand All @@ -1557,6 +1558,7 @@
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match AppStore io.sentry.iOS-SwiftUITests.xctrunner";
"PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*]" = "";
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "SENTRY_UI_TEST $(inherited)";
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
TEST_TARGET_NAME = "iOS-Swift";
Expand Down Expand Up @@ -1723,6 +1725,7 @@
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.iOS-SwiftUITests.xctrunner";
"PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*]" = "";
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "SENTRY_UI_TEST $(inherited)";
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
TEST_TARGET_NAME = "iOS-Swift";
Expand Down Expand Up @@ -1962,6 +1965,7 @@
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.iOS-SwiftUITests.xctrunner";
"PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*]" = "";
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "SENTRY_UI_TEST $(inherited)";
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
TEST_TARGET_NAME = "iOS-Swift";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
buildConfiguration = "Test"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
shouldUseLaunchSchemeArgsEnv = "YES">
shouldUseLaunchSchemeArgsEnv = "YES"
codeCoverageEnabled = "YES">
<Testables>
<TestableReference
skipped = "NO">
Expand Down Expand Up @@ -127,7 +128,7 @@
</CommandLineArgument>
<CommandLineArgument
argument = "--io.sentry.feedback.all-defaults"
isEnabled = "NO">
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "--skip-sentry-init"
Expand Down
85 changes: 55 additions & 30 deletions Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// swiftlint:disable file_length

import Sentry
import UIKit

Expand Down Expand Up @@ -52,7 +54,7 @@
options.enableNetworkBreadcrumbs = enableNetworkBreadcrumbs
options.enableSwizzling = enableSwizzling
options.enableCrashHandler = enableCrashHandling
options.enableTracing = enableTracing

Check warning on line 57 in Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 15 (17.2) Simulator

'enableTracing' is deprecated: Use tracesSampleRate or tracesSampler instead

Check warning on line 57 in Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift iPhone 14 (16.4) Simulator

'enableTracing' is deprecated: Use tracesSampleRate or tracesSampler instead
options.enablePersistingTracesWhenCrashing = true
options.attachScreenshot = enableAttachScreenshot
options.attachViewHierarchy = enableAttachViewHierarchy
Expand Down Expand Up @@ -138,7 +140,6 @@
} else {
config.labelText = "Report Jank"
}
config.widgetAccessibilityLabel = "io.sentry.iOS-Swift.button.report-jank"
config.layoutUIOffset = layoutOffset
} else {
config.autoInject = false
Expand Down Expand Up @@ -211,72 +212,94 @@

func configureHooks(config: SentryUserFeedbackConfiguration) {
config.onFormOpen = {
createHookFile(name: "onFormOpen")
updateHookMarkers(forEvent: "onFormOpen")
}
config.onFormClose = {
createHookFile(name: "onFormClose")
updateHookMarkers(forEvent: "onFormClose")
}
config.onSubmitSuccess = { info in
let name = info["name"] ?? "$shakespearean_insult_name"
let alert = UIAlertController(title: "Thanks?", message: "We have enough jank of our own, we really didn't need yours too, \(name).", preferredStyle: .alert)
alert.addAction(.init(title: "Deal with it 🕶️", style: .default))
UIApplication.shared.delegate?.window??.rootViewController?.present(alert, animated: true)
let jsonData = (try? JSONSerialization.data(withJSONObject: info, options: .sortedKeys)) ?? Data()
createHookFile(name: "onSubmitSuccess", with: jsonData.base64EncodedString())

// if there's a screenshot's Data in this dictionary, JSONSerialization crashes _even though_ there's a `try?`, so we'll write the base64 encoding of it
var infoToWriteToFile = info
if let attachments = info["attachments"] as? [Any], let screenshot = attachments.first as? Data {
infoToWriteToFile["attachments"] = [screenshot.base64EncodedString()]
}

let jsonData = (try? JSONSerialization.data(withJSONObject: infoToWriteToFile, options: .sortedKeys)) ?? Data()
updateHookMarkers(forEvent: "onSubmitSuccess", with: jsonData.base64EncodedString())
}
config.onSubmitError = { error in
let alert = UIAlertController(title: "D'oh", message: "You tried to report jank, and encountered more jank. The jank has you now: \(error).", preferredStyle: .alert)
alert.addAction(.init(title: "Derp", style: .default))
UIApplication.shared.delegate?.window??.rootViewController?.present(alert, animated: true)
let nserror = error as NSError
let missingFieldsSorted = (nserror.userInfo["missing_fields"] as? [String])?.sorted().joined(separator: ";") ?? ""
createHookFile(name: "onSubmitError", with: "\(nserror.domain);\(nserror.code);\(nserror.localizedDescription);\(missingFieldsSorted)")
updateHookMarkers(forEvent: "onSubmitError", with: "\(nserror.domain);\(nserror.code);\(nserror.localizedDescription);\(missingFieldsSorted)")
}
}

func createHookFile(name: String, with contents: String? = nil) {
func updateHookMarkers(forEvent name: String, with contents: String? = nil) {
guard let appSupportDirectory = NSSearchPathForDirectoriesInDomains(.applicationSupportDirectory, .userDomainMask, true).first else {
print("[iOS-Swift] Couldn't retrieve path to application support directory.")
return
}

let fm = FileManager.default
let dir = "\(appSupportDirectory)/io.sentry/feedback"
do {
try fm.createDirectory(atPath: dir, withIntermediateDirectories: true)
} catch {
print("[iOS-Swift] Couldn't create directory structure for user feedback form hook marker files: \(error).")
return
let isDirectory = UnsafeMutablePointer<ObjCBool>.allocate(capacity: 1)
isDirectory.initialize(to: ObjCBool(false))
let exists = fm.fileExists(atPath: dir, isDirectory: isDirectory)
if exists, !isDirectory.pointee.boolValue {
print("[iOS-Swift] Found a file named \(dir) which is not a directory. Removing it...")
do {
try fm.removeItem(atPath: dir)
} catch {
print("[iOS-Swift] Couldn't remove existing file \(dir): \(error).")
return
}
} else if !exists {
do {
try fm.createDirectory(atPath: dir, withIntermediateDirectories: true)
} catch {
print("[iOS-Swift] Couldn't create directory structure for user feedback form hook marker files: \(error).")
return
}
}

let path = "\(dir)/\(name)"
createHookFile(path: "\(dir)/\(name)", contents: contents)

switch name {
case "onFormOpen": removeHookFile(path: "\(dir)/onFormClose")
case "onFormClose": removeHookFile(path: "\(dir)/onFormOpen")
case "onSubmitSuccess": removeHookFile(path: "\(dir)/onSubmitError")
case "onSubmitError": removeHookFile(path: "\(dir)/onSubmitSuccess")
default: fatalError("Unexpected marker file name")
}
}

func createHookFile(path: String, contents: String?) {
if let contents = contents {
do {
try contents.write(to: URL(fileURLWithPath: path), atomically: false, encoding: .utf8)
} catch {
print("[iOS-Swift] Couldn't write contents into user feedback form hook marker file at \(path).")
}
} else if !fm.createFile(atPath: path, contents: nil) {
} else if !FileManager.default.createFile(atPath: path, contents: nil) {
print("[iOS-Swift] Couldn't create user feedback form hook marker file at \(path).")
} else {
print("[iOS-Swift] Created user feedback form hook marker file at \(path).")
}

func removeHookFile(name: String) {
let path = "\(dir)/\(name)"
do {
try fm.removeItem(atPath: path)
} catch {
print("[iOS-Swift] Couldn't remove user feedback form hook marker file \(path): \(error).")
}
}

switch name {
case "onFormOpen": removeHookFile(name: "onFormClose")
case "onFormClose": removeHookFile(name: "onFormOpen")
case "onSubmitSuccess": removeHookFile(name: "onSubmitError")
case "onSubmitError": removeHookFile(name: "onSubmitSuccess")
default: fatalError("Unexpected marker file name")
}

func removeHookFile(path: String) {
do {
try FileManager.default.removeItem(atPath: path)
} catch {
print("[iOS-Swift] Couldn't remove user feedback form hook marker file \(path): \(error).")
}
}
}
Expand Down Expand Up @@ -394,3 +417,5 @@

var enableAppLaunchProfiling: Bool { args.contains("--profile-app-launches") }
}

// swiftlint:enable file_length
Loading
Loading