-
-
Notifications
You must be signed in to change notification settings - Fork 317
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
feat(user feedback): initial API #4284
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4284 +/- ##
=============================================
- Coverage 91.697% 91.556% -0.142%
=============================================
Files 622 627 +5
Lines 50502 50616 +114
Branches 18251 18347 +96
=============================================
+ Hits 46309 46342 +33
- Misses 4102 4182 +80
- Partials 91 92 +1
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
…gling autoInject, with a sample workflow for manual form display
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.
Looking good, some comments.
Please, move the Swift files to "Swift/Integrations/FeedbackUser", also in the project organization.
…into the widget config class
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.
Maybe my comments dont' make sense if we're consider "user feedback" as the behavior of popping the modal on shake and all the modal settings. But since we've had an API (captureUserFeedback
) for many years, I find that it might be confusing now with the new capabilities.
I wonder if we can find a better way to differentiate between the APIs to capture feedback (new captureFeedback since we're deprecating
captureUserFeedback`) and any automatic behavior like poping up a modal on crash or on shake.
Samples/iOS-Swift/iOS-Swift/Profiling/NSObject+SentryAppSetup.m
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackFormConfiguration.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackFormConfiguration.swift
Outdated
Show resolved
Hide resolved
...ces/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackThemeConfiguration.swift
Outdated
Show resolved
Hide resolved
...es/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackWidgetConfiguration.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/UserFeedback/SentryUserFeedbackIntegration.swift
Outdated
Show resolved
Hide resolved
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.
This looks great, the biggest questions are how do we name the root config, do we remove user from the naming, what is the widget exactly and what is a form. We can also discuss this on a call cause these discussions could take forever on GH. The rest LGTM.
...es/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackWidgetConfiguration.swift
Show resolved
Hide resolved
...es/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackWidgetConfiguration.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackFormConfiguration.swift
Show resolved
Hide resolved
Sources/Swift/Integrations/UserFeedback/Configuration/SentryUserFeedbackConfiguration.swift
Outdated
Show resolved
Hide resolved
cebfd4d
to
2bef25e
Compare
92ce272
to
131c10b
Compare
@bruno-garcia I do have the shake trigger in here already, but popping a dialog on crash detection is a good one too 👍🏻 Though I'd echo @philipphofmann 's comment to keep scope narrow for v1 and follow on with that. |
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.
The user-facing API, LGTM.
Pushed a commit to remove this from public access while we merge intermediate PRs |
An initial draft of the API we'll offer to customers for the new User Feedback widget and managed form, for #4269, including an example of how it would look to configure it, in iOS-Swift.
Docs PR: getsentry/sentry-docs#11191
#skip-changelog