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

Add screenshot to SentryFeedbackWidget #2369

Merged
merged 11 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,28 @@

## Unreleased

### Features

- Add screenshot to `SentryFeedbackWidget` ([#2369](https://github.com/getsentry/sentry-dart/pull/2369))
- Use `SentryFlutter.captureScreenshot` to create a screenshot attachment
- Call `SentryFeedbackWidget` with this attachment to add it to the user feedback

```dart
final id = await Sentry.captureMessage('UserFeedback');
final screenshot = await SentryFlutter.captureScreenshot();

Navigator.push(
context,
MaterialPageRoute(
builder: (context) => SentryFeedbackWidget(
associatedEventId: id,
screenshot: screenshot,
),
fullscreenDialog: true,
),
);
```

### Enhancements

- Cache parsed DSN ([#2365](https://github.com/getsentry/sentry-dart/pull/2365))
Expand Down
6 changes: 4 additions & 2 deletions flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -502,12 +502,14 @@ class MainScaffold extends StatelessWidget {
TooltipButton(
onPressed: () async {
final id = await Sentry.captureMessage('UserFeedback');
final screenshot = await SentryFlutter.captureScreenshot();

if (!context.mounted) return;
Navigator.push(
context,
MaterialPageRoute(
builder: (context) =>
SentryFeedbackWidget(associatedEventId: id),
builder: (context) => SentryFeedbackWidget(
associatedEventId: id, screenshot: screenshot),
fullscreenDialog: true,
),
);
Expand Down
19 changes: 11 additions & 8 deletions flutter/lib/src/event_processor/screenshot_event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import 'dart:math';
import 'dart:typed_data';
import 'dart:ui';

import 'package:flutter/rendering.dart';
import 'package:meta/meta.dart';
import 'package:sentry/sentry.dart';
import '../screenshot/sentry_screenshot_widget.dart';
import '../sentry_flutter_options.dart';
import 'package:flutter/rendering.dart';
import '../renderer/renderer.dart';
import 'package:flutter/widgets.dart' as widget;

Expand All @@ -15,10 +16,6 @@ class ScreenshotEventProcessor implements EventProcessor {

ScreenshotEventProcessor(this._options);

/// This is true when the SentryWidget is in the view hierarchy
bool get _hasSentryScreenshotWidget =>
sentryScreenshotWidgetGlobalKey.currentContext != null;

@override
Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
if (event is SentryTransaction) {
Expand All @@ -27,9 +24,14 @@ class ScreenshotEventProcessor implements EventProcessor {

if (event.exceptions == null &&
event.throwable == null &&
_hasSentryScreenshotWidget) {
SentryScreenshotWidget.isMounted) {
return event;
}

if (event.type == 'feedback') {
return event; // No need to attach screenshot of feedback form.
}

final beforeScreenshot = _options.beforeScreenshot;
if (beforeScreenshot != null) {
try {
Expand Down Expand Up @@ -75,14 +77,15 @@ class ScreenshotEventProcessor implements EventProcessor {
return event;
}

final bytes = await _createScreenshot();
final bytes = await createScreenshot();
if (bytes != null) {
hint.screenshot = SentryAttachment.fromScreenshotData(bytes);
}
return event;
}

Future<Uint8List?> _createScreenshot() async {
@internal
Future<Uint8List?> createScreenshot() async {
try {
final renderObject =
sentryScreenshotWidgetGlobalKey.currentContext?.findRenderObject();
Expand Down
14 changes: 11 additions & 3 deletions flutter/lib/src/feedback/sentry_feedback_widget.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class SentryFeedbackWidget extends StatefulWidget {
this.isRequiredLabel = '(required)',
this.isNameRequired = false,
this.isEmailRequired = false,
this.screenshot,
}) : assert(associatedEventId != const SentryId.empty()),
_hub = hub ?? HubAdapter();

Expand All @@ -45,6 +46,8 @@ class SentryFeedbackWidget extends StatefulWidget {
final bool isNameRequired;
final bool isEmailRequired;

final SentryAttachment? screenshot;

@override
_SentryFeedbackWidgetState createState() => _SentryFeedbackWidgetState();
}
Expand Down Expand Up @@ -197,7 +200,12 @@ class _SentryFeedbackWidgetState extends State<SentryFeedbackWidget> {
name: _nameController.text,
associatedEventId: widget.associatedEventId,
);
await _captureFeedback(feedback);
Hint? hint;
final screenshot = widget.screenshot;
if (screenshot != null) {
hint = Hint.withScreenshot(screenshot);
}
await _captureFeedback(feedback, hint);

bool mounted;
try {
Expand Down Expand Up @@ -246,7 +254,7 @@ class _SentryFeedbackWidgetState extends State<SentryFeedbackWidget> {
return null;
}

Future<SentryId> _captureFeedback(SentryFeedback feedback) {
return widget._hub.captureFeedback(feedback);
Future<SentryId> _captureFeedback(SentryFeedback feedback, Hint? hint) {
return widget._hub.captureFeedback(feedback, hint: hint);
}
}
4 changes: 4 additions & 0 deletions flutter/lib/src/screenshot/sentry_screenshot_widget.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ class SentryScreenshotWidget extends StatefulWidget {

@override
_SentryScreenshotWidgetState createState() => _SentryScreenshotWidgetState();

/// This is true when the [SentryScreenshotWidget] is in the widget tree.
static bool get isMounted =>
sentryScreenshotWidgetGlobalKey.currentContext != null;
}

class _SentryScreenshotWidgetState extends State<SentryScreenshotWidget> {
Expand Down
31 changes: 31 additions & 0 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import 'event_processor/flutter_enricher_event_processor.dart';
import 'event_processor/flutter_exception_event_processor.dart';
import 'event_processor/platform_exception_event_processor.dart';
import 'event_processor/screenshot_event_processor.dart';
import 'event_processor/url_filter/url_filter_event_processor.dart';
import 'event_processor/widget_event_processor.dart';
import 'file_system_transport.dart';
Expand Down Expand Up @@ -284,6 +285,36 @@
}
}

/// Uses [SentryScreenshotWidget] to capture the current screen as a
/// [SentryAttachment].
static Future<SentryAttachment?> captureScreenshot() async {

Check warning on line 290 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L290

Added line #L290 was not covered by tests
// ignore: invalid_use_of_internal_member
final options = Sentry.currentHub.options;
if (!SentryScreenshotWidget.isMounted) {
options.logger(

Check warning on line 294 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L292-L294

Added lines #L292 - L294 were not covered by tests
SentryLevel.debug,
'SentryScreenshotWidget could not be found in the widget tree.',
);
return null;
}
final processors =
options.eventProcessors.whereType<ScreenshotEventProcessor>();
if (processors.isEmpty) {
options.logger(

Check warning on line 303 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L301-L303

Added lines #L301 - L303 were not covered by tests
SentryLevel.debug,
'ScreenshotEventProcessor could not be found.',
);
return null;
}
final processor = processors.first;
final bytes = await processor.createScreenshot();

Check warning on line 310 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L309-L310

Added lines #L309 - L310 were not covered by tests
if (bytes != null) {
return SentryAttachment.fromScreenshotData(bytes);

Check warning on line 312 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L312

Added line #L312 was not covered by tests
} else {
return null;
}
}

@internal
static SentryNativeBinding? get native => _native;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,36 @@ void main() {
added: true, isWeb: false, expectedMaxWidthOrHeight: widthOrHeight);
});

testWidgets('does not add screenshot for feedback events', (tester) async {
await tester.runAsync(() async {
final sut = fixture.getSut(null, false);

await tester.pumpWidget(
SentryScreenshotWidget(
child: Text('Catching Pokémon is a snap!',
textDirection: TextDirection.ltr),
),
);

final feedback = SentryFeedback(
message: 'message',
contactEmail: '[email protected]',
name: 'Joe Dirt',
associatedEventId: null,
);
final feedbackEvent = SentryEvent(
type: 'feedback',
contexts: Contexts(feedback: feedback),
level: SentryLevel.info,
);

final hint = Hint();
await sut.apply(feedbackEvent, hint);

expect(hint.screenshot, isNull);
});
});

group('beforeScreenshot', () {
testWidgets('does add screenshot if beforeScreenshot returns true',
(tester) async {
Expand Down
38 changes: 38 additions & 0 deletions flutter/test/feedback/sentry_feedback_widget_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,44 @@ void main() {
fixture = Fixture();
});

testWidgets('does add screenshot attachment to hint', (tester) async {
final screenshot = SentryAttachment.fromIntList([0, 0, 0, 0], 'test.png');

await fixture.pumpFeedbackWidget(
tester,
(hub) => SentryFeedbackWidget(
hub: hub,
screenshot: screenshot,
),
);

when(fixture.hub.captureFeedback(
any,
hint: anyNamed('hint'),
withScope: anyNamed('withScope'),
)).thenAnswer(
(_) async => SentryId.fromId('1988bb1b6f0d4c509e232f0cb9aaeaea'));

await tester.enterText(
find.byKey(ValueKey('sentry_feedback_name_textfield')),
"fixture-name");
await tester.enterText(
find.byKey(ValueKey('sentry_feedback_email_textfield')),
"fixture-email");
await tester.enterText(
find.byKey(ValueKey('sentry_feedback_message_textfield')),
"fixture-message");
await tester.tap(find.text('Send Bug Report'));
await tester.pumpAndSettle();

verify(fixture.hub.captureFeedback(
any,
hint: argThat(predicate<Hint>((hint) => hint.screenshot == screenshot),
named: 'hint'),
withScope: anyNamed('withScope'),
)).called(1);
});

testWidgets('does call hub captureFeedback on submit', (tester) async {
await fixture.pumpFeedbackWidget(
tester,
Expand Down
2 changes: 1 addition & 1 deletion min_version_test/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class _MyHomePageState extends State<MyHomePage> {
Text(
'$_counter',
// ignore: deprecated_member_use
style: Theme.of(context).textTheme.headline4,
style: Theme.of(context).textTheme.headlineMedium,
),
],
),
Expand Down
Loading