Skip to content

Commit

Permalink
Clean up styling for analytics prompt (#7539)
Browse files Browse the repository at this point in the history
* Clean up styling for analytics prompt

* fix comment
  • Loading branch information
kenzieschmoll authored Apr 8, 2024
1 parent 8ad24e4 commit aa9ddf5
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';

import 'package:flutter/foundation.dart';

import '../development_helpers.dart';
import '../dtd_manager_extensions.dart';
import '../globals.dart';

Expand All @@ -22,7 +23,8 @@ Future<AnalyticsController> get analyticsController async {
var shouldShowConsentMessage = false;
try {
enabled = await ga.isAnalyticsEnabled();
shouldShowConsentMessage = await ga.shouldShowAnalyticsConsentMessage();
shouldShowConsentMessage = debugShowAnalyticsConsentMessage ||
await ga.shouldShowAnalyticsConsentMessage();
} catch (_) {
// Ignore issues if analytics could not be initialized.
}
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools_app/lib/src/shared/analytics/gtags.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ class GTag {
String eventName, {
required GtagEvent Function() gaEventProvider,
}) async {
if (debugAnalytics || (kReleaseMode && await ga.isAnalyticsEnabled())) {
if (debugSendAnalytics || (kReleaseMode && await ga.isAnalyticsEnabled())) {
_gTagCommandName(_event, eventName, gaEventProvider());
}
}

static void exception({
required GtagException Function() gaExceptionProvider,
}) async {
if (debugAnalytics || (kReleaseMode && await ga.isAnalyticsEnabled())) {
if (debugSendAnalytics || (kReleaseMode && await ga.isAnalyticsEnabled())) {
_gTagCommandName(_event, _exception, gaExceptionProvider());
}
}
Expand Down
72 changes: 39 additions & 33 deletions packages/devtools_app/lib/src/shared/analytics/prompt.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ class _AnalyticsPromptState extends State<AnalyticsPrompt>
@override
Widget build(BuildContext context) {
final theme = Theme.of(context);
final textTheme = theme.textTheme;

return ValueListenableBuilder<bool>(
valueListenable: controller.shouldPrompt,
builder: (context, showPrompt, child) {
Expand Down Expand Up @@ -72,19 +70,17 @@ class _AnalyticsPromptState extends State<AnalyticsPrompt>
children: [
Text(
'Send usage statistics for DevTools?',
style: textTheme.headlineSmall,
style: theme.boldTextStyle,
),
IconButton.outlined(
icon: const Icon(Icons.close),
IconButton(
icon: Icon(Icons.close, size: actionsIconSize),
onPressed: controller.hidePrompt,
),
],
),
const Padding(
padding: EdgeInsets.only(top: defaultSpacing),
),
_analyticsDescription(textTheme),
const SizedBox(height: denseRowSpacing),
const SizedBox(height: denseSpacing),
_analyticsDescription(theme),
const SizedBox(height: defaultSpacing),
_actionButtons(),
],
),
Expand All @@ -93,44 +89,45 @@ class _AnalyticsPromptState extends State<AnalyticsPrompt>
);
}

Widget _analyticsDescription(TextTheme textTheme) {
Widget _analyticsDescription(ThemeData theme) {
final consentMessageRegExpResults =
parseAnalyticsConsentMessage(controller.consentMessage);
parseAnalyticsConsentMessage(controller.consentMessage)
?.map((e) => adjustLineBreaks(e))
.toList();

// When failing to parse the consent message, fallback to
// displaying the consent message in its regular form
// When failing to parse the consent message, fallback to displaying the
// consent message in its regular form.
if (consentMessageRegExpResults == null) {
return RichText(
text: TextSpan(
return SelectableText.rich(
TextSpan(
children: [
TextSpan(
text: controller.consentMessage,
style: textTheme.titleMedium,
text: adjustLineBreaks(controller.consentMessage),
style: theme.regularTextStyle,
),
],
),
);
}

return RichText(
text: TextSpan(
return SelectableText.rich(
TextSpan(
children: [
TextSpan(
text: consentMessageRegExpResults[0],
style: textTheme.titleMedium,
style: theme.regularTextStyle,
),
LinkTextSpan(
link: Link(
display: consentMessageRegExpResults[1],
url: consentMessageRegExpResults[1],
),
context: context,
style:
textTheme.titleMedium?.copyWith(color: const Color(0xFF54C1EF)),
style: theme.linkTextStyle,
),
TextSpan(
text: consentMessageRegExpResults[2],
style: textTheme.titleMedium,
style: theme.regularTextStyle,
),
],
),
Expand All @@ -141,23 +138,21 @@ class _AnalyticsPromptState extends State<AnalyticsPrompt>
return Row(
crossAxisAlignment: CrossAxisAlignment.end,
children: [
ElevatedButton(
DevToolsButton(
label: 'No thanks',
onPressed: () {
// This will also hide the prompt.
unawaited(controller.toggleAnalyticsEnabled(false));
},
style: ElevatedButton.styleFrom(backgroundColor: Colors.grey),
child: const Text('No thanks.'),
),
const Padding(
padding: EdgeInsets.only(left: defaultSpacing),
),
ElevatedButton(
const SizedBox(width: defaultSpacing),
DevToolsButton(
label: 'Sounds good!',
elevated: true,
onPressed: () {
unawaited(controller.toggleAnalyticsEnabled(true));
controller.hidePrompt();
},
child: const Text('Sounds good!'),
),
],
);
Expand All @@ -168,9 +163,10 @@ class _AnalyticsPromptState extends State<AnalyticsPrompt>
/// `package:unified_analytics` so that the URL can be
/// separated from the block of text so that we can have a
/// hyperlink in the displayed consent message.
@visibleForTesting
List<String>? parseAnalyticsConsentMessage(String consentMessage) {
final results = <String>[];
final RegExp pattern =
final pattern =
RegExp(r'^([\S\s]*)(https?:\/\/[^\s]+)(\)\.)$', multiLine: true);

final matches = pattern.allMatches(consentMessage);
Expand All @@ -191,3 +187,13 @@ List<String>? parseAnalyticsConsentMessage(String consentMessage) {

return results;
}

/// Replaces single line breaks with spaces so that the text [value] can be
/// displayed in a responsive UI and does not have fixed line breaks that do not
/// match the width of the view.
@visibleForTesting
String adjustLineBreaks(String value) {
final pattern =
RegExp(r'(?<!\r\n|\r|\n)(\r\n|\r|\n)(?!\r\n|\r|\n)', multiLine: true);
return value.replaceAll(pattern, ' ');
}
10 changes: 7 additions & 3 deletions packages/devtools_app/lib/src/shared/development_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,15 @@ final _log = Logger('dev_helpers');
String? get debugDtdUri => kReleaseMode ? null : _debugDtdUri;
String? _debugDtdUri;

/// Enable this flag to debug analytics when DevTools is run in debug or profile
/// mode, otherwise analytics will only be sent in release builds.
/// Enable this flag to send and debug analytics when DevTools is run in debug
/// or profile mode, otherwise analytics will only be sent in release builds.
///
/// `ga.isAnalyticsEnabled()` still must return true for analytics to be sent.
bool debugAnalytics = false;
bool debugSendAnalytics = false;

/// Enable this flag to always show the analytics consent message, regardless
/// of whether any other conditions are met.
bool debugShowAnalyticsConsentMessage = false;

/// Whether to build DevTools for conveniently debugging DevTools extensions.
///
Expand Down
21 changes: 20 additions & 1 deletion packages/devtools_app/test/shared/analytics_prompt_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,25 @@ void main() {
expect(result, hasLength(3));
});

test('Unit test adjustLineBreaks with consent message', () {
final result = adjustLineBreaks(ua.kToolsMessage);
const expected =
'The {{ toolDescription }} uses Google Analytics to report usage and diagnostic '
'data along with package dependencies, and crash reporting to send basic crash '
'reports. This data is used to help improve the Dart platform, Flutter framework, '
'and related tools.'
'\n\n'
'Telemetry is not sent on the very first run. To disable reporting of telemetry, '
'run this terminal command:'
'\n\n'
' {{ toolName }} --disable-analytics'
'\n\n'
'If you opt out of telemetry, an opt-out event will be sent, and then no further '
'information will be sent. This data is collected in accordance with the Google '
'Privacy Policy (https://policies.google.com/privacy). ';
expect(result, expected);
});

group('AnalyticsPrompt', () {
setUp(() {
didCallEnableAnalytics = false;
Expand Down Expand Up @@ -302,7 +321,7 @@ void main() {
expect(controller.analyticsEnabled.value, isTrue);
expect(didCallEnableAnalytics, isTrue);

final noThanksFinder = find.text('No thanks.');
final noThanksFinder = find.text('No thanks');
expect(noThanksFinder, findsOneWidget);
await tester.tap(noThanksFinder);
await tester.pumpAndSettle();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import 'package:flutter_test/flutter_test.dart';
void main() {
test('debug flags are false', () {
expect(debugDtdUri, isNull);
expect(debugAnalytics, isFalse);
expect(debugSendAnalytics, isFalse);
expect(debugShowAnalyticsConsentMessage, isFalse);
expect(debugDevToolsExtensions, isFalse);
expect(debugSurvey, isFalse);
expect(debugPerfettoTraceProcessing, isFalse);
Expand Down

0 comments on commit aa9ddf5

Please sign in to comment.