Skip to content

fix: Show success message after feedback submission#3609

Open
denrase wants to merge 5 commits intomainfrom
feat/feedback-success-message
Open

fix: Show success message after feedback submission#3609
denrase wants to merge 5 commits intomainfrom
feat/feedback-success-message

Conversation

@denrase
Copy link
Copy Markdown
Collaborator

@denrase denrase commented Mar 30, 2026

📜 Description

Display a success screen with successMessageText after feedback is submitted. The form is replaced with a checkmark icon and success message that auto-dismisses after 5 seconds or on tap. Adds successColor and onSubmitSuccess callback to SentryFeedbackOptions.

Success Message
Bildschirmfoto 2026-03-30 um 11 20 35

💡 Motivation and Context

Closes #3583

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

denrase added 2 commits March 30, 2026 11:20
Display a success screen with successMessageText after feedback is submitted. The form is replaced with a checkmark icon and success message that auto-dismisses after 5 seconds or on tap. Adds `successColor` and `onSubmitSuccess` callback to `SentryFeedbackOptions`.
@denrase denrase marked this pull request as ready for review March 30, 2026 09:23
@denrase denrase requested a review from buenaflor as a code owner March 30, 2026 09:23
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 496.35 ms 485.24 ms -11.10 ms
Size 14.31 MiB 15.49 MiB 1.19 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
cfca825 417.36 ms 393.37 ms -23.99 ms
51520fc 351.89 ms 349.79 ms -2.10 ms
75284dc 512.39 ms 530.87 ms 18.48 ms
a909995 383.55 ms 370.78 ms -12.77 ms
f761369 462.73 ms 563.80 ms 101.06 ms
575ebaa 478.00 ms 585.76 ms 107.76 ms
a5b28db 383.85 ms 387.65 ms 3.80 ms
e5b87f8 371.22 ms 377.22 ms 6.00 ms
c8596a6 474.00 ms 492.96 ms 18.96 ms
79f6b41 469.66 ms 525.90 ms 56.24 ms

App size

Revision Plain With Sentry Diff
cfca825 14.09 MiB 15.28 MiB 1.19 MiB
51520fc 13.93 MiB 15.18 MiB 1.25 MiB
75284dc 13.93 MiB 14.93 MiB 1.00 MiB
a909995 14.09 MiB 15.28 MiB 1.19 MiB
f761369 6.54 MiB 7.70 MiB 1.16 MiB
575ebaa 6.54 MiB 7.69 MiB 1.15 MiB
a5b28db 13.93 MiB 15.18 MiB 1.25 MiB
e5b87f8 13.93 MiB 15.18 MiB 1.25 MiB
c8596a6 6.54 MiB 7.53 MiB 1015.27 KiB
79f6b41 6.54 MiB 7.69 MiB 1.15 MiB

Previous results on branch: feat/feedback-success-message

Startup times

Revision Plain With Sentry Diff
04e57cb 364.44 ms 361.00 ms -3.44 ms

App size

Revision Plain With Sentry Diff
04e57cb 14.31 MiB 15.49 MiB 1.19 MiB

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.08 ms 1245.55 ms 7.47 ms
Size 5.73 MiB 6.17 MiB 455.50 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
7cfbbd6 1270.63 ms 1285.36 ms 14.72 ms
e2d675d 1238.48 ms 1242.76 ms 4.28 ms
e0c8591 1259.85 ms 1257.31 ms -2.54 ms
1ce780b 1252.49 ms 1256.17 ms 3.68 ms
d0aa4b6 1268.23 ms 1268.39 ms 0.15 ms
3801d52 1267.76 ms 1266.10 ms -1.65 ms
c97f488 1247.40 ms 1252.13 ms 4.73 ms
fd88186 1255.06 ms 1252.76 ms -2.30 ms
aeb02f2 1244.29 ms 1256.55 ms 12.26 ms
c26ed0a 1244.11 ms 1263.85 ms 19.75 ms

App size

Revision Plain With Sentry Diff
7cfbbd6 7.86 MiB 9.44 MiB 1.58 MiB
e2d675d 7.86 MiB 9.44 MiB 1.58 MiB
e0c8591 5.53 MiB 5.96 MiB 444.86 KiB
1ce780b 5.66 MiB 6.10 MiB 451.58 KiB
d0aa4b6 5.53 MiB 6.02 MiB 502.04 KiB
3801d52 5.73 MiB 6.17 MiB 455.54 KiB
c97f488 5.73 MiB 6.17 MiB 455.48 KiB
fd88186 5.53 MiB 6.00 MiB 479.94 KiB
aeb02f2 7.86 MiB 9.44 MiB 1.58 MiB
c26ed0a 5.53 MiB 5.97 MiB 453.76 KiB

Previous results on branch: feat/feedback-success-message

Startup times

Revision Plain With Sentry Diff
04e57cb 1246.67 ms 1248.35 ms 1.67 ms
f289ae3 1258.83 ms 1258.98 ms 0.15 ms

App size

Revision Plain With Sentry Diff
04e57cb 5.73 MiB 6.17 MiB 455.48 KiB
f289ae3 5.73 MiB 6.17 MiB 455.48 KiB

@sentry
Copy link
Copy Markdown

sentry bot commented Mar 30, 2026

Sentry Build Distribution

App Name App ID Version Configuration Install Page
sentry_flutter_example io.sentry.flutter.sample 9.16.0 (1) Release Install Build

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 96.21212% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.83%. Comparing base (5b3a9e9) to head (6f5601c).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...utter/lib/src/feedback/sentry_feedback_widget.dart 96.21% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3609      +/-   ##
==========================================
+ Coverage   86.83%   91.83%   +4.99%     
==========================================
  Files         320      102     -218     
  Lines       10789     3502    -7287     
==========================================
- Hits         9369     3216    -6153     
+ Misses       1420      286    -1134     
Flag Coverage Δ
sentry ?
sentry_dio ?
sentry_drift ?
sentry_file ?
sentry_firebase_remote_config 100.00% <ø> (ø)
sentry_flutter 91.41% <96.21%> (+0.40%) ⬆️
sentry_hive ?
sentry_isar ?
sentry_link ?
sentry_logging ?
sentry_sqflite ?
sentry_supabase 97.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

if (mounted && _submitted) {
_dismiss(pendingAssociatedEventId: false);
}
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success message shown even when feedback capture fails

Medium Severity

The _submit method shows the success UI and invokes onSubmitSuccess without checking whether _captureFeedback actually succeeded. captureFeedback on the hub returns SentryId.empty() when the instance is disabled or when capture throws an exception. The established SDK pattern (visible in example_web/web/main.dart) is to check sentryId != SentryId.empty() before treating a capture as successful. Without this guard, users see "Thank you for your report!" and the onSubmitSuccess callback fires even when their feedback was silently dropped.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, that we effectively do not get negative. We send an envelope to JS/Android/iOS downstream SDK. The modal is closed in any case.

Copy link
Copy Markdown
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denrase PTAL at AI review first

@denrase denrase requested a review from buenaflor March 31, 2026 14:21
Comment on lines +423 to +432
if (!_formKey.currentState!.validate()) {
return;
}

final feedback = SentryFeedback(
message: _messageController.text,
contactEmail: _emailController.text,
name: _nameController.text,
associatedEventId: widget.associatedEventId,
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The _submit() method lacks a guard against concurrent calls, allowing multiple submissions if the user taps the submit button quickly before the async operation completes.
Severity: MEDIUM

Suggested Fix

Introduce a boolean flag like _isSubmitting to guard the _submit method. Set it to true at the beginning of the method and false after the submission completes. Check this flag at the start of the method and return early if a submission is already in progress.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/flutter/lib/src/feedback/sentry_feedback_widget.dart#L423-L432

Potential issue: The `_submit()` method is an `async` function that can be invoked
multiple times before the first call completes, creating a race condition. If a user
taps the submit button twice in quick succession, `_captureFeedback()` will be called
twice, resulting in duplicate feedback reports being sent. Additionally, when the second
submission completes, it creates a new `Timer` and assigns it to `_successTimer`,
overwriting the reference to the first timer without cancelling it. This orphans the
first timer, causing a resource leak, and leads to the `onSubmitSuccess` callback being
invoked multiple times.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

}
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timer not cancelled in _dismiss risks double navigation

Low Severity

_dismiss does not cancel _successTimer. The timer is only cancelled in dispose(). If a user taps the success screen (or close button) to dismiss very close to the 5-second auto-dismiss deadline, both the user action and the timer callback can call _dismiss, resulting in Navigator.maybePop being invoked twice during the same pop transition. Cancelling _successTimer at the start of _dismiss would prevent this race.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot (Root)

Copy link
Copy Markdown
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed let's do that but let's make the out of the box success widget not the whole screen, please look into what's idiomatic for Flutter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feedback successMessageText never shown

2 participants