Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:logging/logging.dart';

import '../../service/service_extensions.dart' as extensions;
import '../../service/service_registrations.dart' as registrations;
import '../../service/view_metrics_override_client.dart';
import '../../shared/framework/screen.dart';
import '../../shared/framework/screen_controllers.dart';
import '../../shared/globals.dart';
Expand Down Expand Up @@ -46,15 +47,12 @@ class AccessibilityController extends DevToolsScreenController
void init() {
super.init();
_initServiceExtensionStates();
_initViewMetricsOverrides();
_initSemanticsTree();
}

void _initListeners() {
addAutoDisposeListener(brightness, _onBrightnessChanged);
addAutoDisposeListener(textScale, _onTextScaleChanged);
addAutoDisposeListener(boldText, _onBoldTextChanged);
addAutoDisposeListener(screenReader, _onScreenReaderChanged);
addAutoDisposeListener(highContrast, _onHighContrastChanged);
addAutoDisposeListener(semanticsRoots, _onSemanticsRootsChanged);
}

Expand Down Expand Up @@ -123,21 +121,78 @@ class AccessibilityController extends DevToolsScreenController
);
}

void _onTextScaleChanged() {
// TODO(hannah-hyj): Implement VM service extension call for text scale override.
}
// The metrics of `ext.flutter.viewMetricsOverride` that the overrides pane
// controls. Brightness stays on `ext.flutter.brightnessOverride`.
static const _textScalingMetric = 'textScaling';
static const _boldTextMetric = 'boldText';
static const _accessibleNavigationMetric = 'accessibleNavigation';
static const _highContrastMetric = 'highContrast';

/// The client that overrides the text scale, bold text, screen reader and
/// high contrast settings of the app, or null before [init].
ViewMetricsOverrideClient? _viewMetricsOverrideClient;

void _initViewMetricsOverrides() {
final client = _viewMetricsOverrideClient = ViewMetricsOverrideClient();
void updateFromApp() {
final overrides = client.overrides.value;
_overridesAvailable.value = client.available.value;
_textScale.value = _textScaleFactor(overrides[_textScalingMetric]);
_boldText.value = overrides[_boldTextMetric] == true;
_screenReader.value = overrides[_accessibleNavigationMetric] == true;
_highContrast.value = overrides[_highContrastMetric] == true;
}

void _onBoldTextChanged() {
// TODO(hannah-hyj): Implement VM service extension call for bold text override.
updateFromApp();
addAutoDisposeListener(client.available, updateFromApp);
addAutoDisposeListener(client.overrides, updateFromApp);
}

void _onScreenReaderChanged() {
// TODO(hannah-hyj): Implement VM service extension call for screen reader / semantics debugger.
// e.g. using 'ext.flutter.showSemanticsDebugger'.
/// Overrides the text scale factor of the app with [factor], or stops
/// overriding it if [factor] is null.
///
/// The override scales every font size linearly by [factor].
Future<void> setTextScale(double? factor) => _setOverride(
_textScalingMetric,
factor == null
? null
: [
[1, factor],
],
);

/// Makes the app use bold text, or the bold text setting of the platform
/// again if [enabled] is false.
Future<void> setBoldText(bool enabled) =>
_setFlagOverride(_boldTextMetric, enabled);

/// Makes the app behave as though a screen reader is on, or follow the
/// platform again if [enabled] is false.
Future<void> setScreenReader(bool enabled) =>
_setFlagOverride(_accessibleNavigationMetric, enabled);

/// Makes the app use high contrast, or the high contrast setting of the
/// platform again if [enabled] is false.
Future<void> setHighContrast(bool enabled) =>
_setFlagOverride(_highContrastMetric, enabled);

// Turning an override off stops overriding the flag rather than overriding
// it to false, so that the app goes back to the setting of the platform.
Future<void> _setFlagOverride(String metric, bool enabled) =>
_setOverride(metric, enabled ? true : null);

Future<void> _setOverride(String metric, Object? value) async {
await _viewMetricsOverrideClient?.updateOverrides({metric: value});
}
Comment on lines +184 to 186

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Catching the exception here and pushing a notification via notificationService provides a much better user experience. It ensures that if the service extension call fails, the user is notified immediately rather than the UI silently failing to apply the override.

Suggested change
Future<void> _setOverride(String metric, Object? value) async {
await _viewMetricsOverrideClient?.updateOverrides({metric: value});
}
Future<void> _setOverride(String metric, Object? value) async {
try {
await _viewMetricsOverrideClient?.updateOverrides({metric: value});
} catch (e) {
notificationService.push('Failed to set override: $e');
}
}
References
  1. Prioritize logic, performance on the UI thread, and architectural consistency. (link)


void _onHighContrastChanged() {
// TODO(hannah-hyj): Implement VM service extension call for high contrast override.
/// Returns the ratio of the first point of [textScaling], which is the text
/// scale factor that the app reports, or null if text scale is not
/// overridden.
static double? _textScaleFactor(Object? textScaling) {
if (textScaling case [[final num unscaled, final num scaled], ...]) {
return scaled / unscaled;
}
return null;
}
Comment on lines +191 to 196

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Defensively check if unscaled is zero to avoid division by zero, which would result in double.infinity or double.nan and potentially cause layout or rendering issues in the UI.

Suggested change
static double? _textScaleFactor(Object? textScaling) {
if (textScaling case [[final num unscaled, final num scaled], ...]) {
return scaled / unscaled;
}
return null;
}
static double? _textScaleFactor(Object? textScaling) {
if (textScaling case [[final num unscaled, final num scaled], ...]) {
return unscaled == 0 ? null : scaled / unscaled;
}
return null;
}
References
  1. Prioritize logic, performance on the UI thread, and architectural consistency. (link)


@override
Expand All @@ -147,10 +202,33 @@ class AccessibilityController extends DevToolsScreenController
final brightness = ValueNotifier<BrightnessOverride>(
BrightnessOverride.system,
);
final textScale = ValueNotifier<double>(1.0);
final boldText = ValueNotifier<bool>(false);
final screenReader = ValueNotifier<bool>(false);
final highContrast = ValueNotifier<bool>(false);

/// Whether the connected app supports the text scale, bold text, screen
/// reader and high contrast overrides.
///
/// See [ViewMetricsOverrideClient.available] for the apps that do.
ValueListenable<bool> get overridesAvailable => _overridesAvailable;
final _overridesAvailable = ValueNotifier<bool>(false);

/// The text scale factor that the app is overridden to, or null if the app
/// uses the text scale that the platform reports.
ValueListenable<double?> get textScale => _textScale;
final _textScale = ValueNotifier<double?>(null);

/// Whether the app is overridden to use bold text.
ValueListenable<bool> get boldText => _boldText;
final _boldText = ValueNotifier<bool>(false);

/// Whether the app is overridden to behave as though a screen reader is on.
///
/// This overrides `AccessibilityFeatures.accessibleNavigation`, which
/// changes how widgets behave. It does not start a screen reader.
ValueListenable<bool> get screenReader => _screenReader;
final _screenReader = ValueNotifier<bool>(false);

/// Whether the app is overridden to use high contrast.
ValueListenable<bool> get highContrast => _highContrast;
final _highContrast = ValueNotifier<bool>(false);

final semanticsRoots = ValueNotifier<List<SemanticsNodeModel>>([]);
final selectedSemanticsNode = ValueNotifier<SemanticsNodeModel?>(null);
Expand Down Expand Up @@ -321,11 +399,13 @@ class AccessibilityController extends DevToolsScreenController
@override
void dispose() {
unawaited(_disposeSemanticsOnApp());
_viewMetricsOverrideClient?.dispose();
brightness.dispose();
textScale.dispose();
boldText.dispose();
screenReader.dispose();
highContrast.dispose();
_overridesAvailable.dispose();
_textScale.dispose();
_boldText.dispose();
_screenReader.dispose();
_highContrast.dispose();
semanticsRoots.dispose();
selectedSemanticsNode.dispose();
semanticsTreeLoading.dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.

import 'dart:async';

import 'package:devtools_app_shared/ui.dart';
import 'package:flutter/foundation.dart';
import 'package:material_ui/material_ui.dart';

import '../../shared/globals.dart';
Expand Down Expand Up @@ -38,25 +41,7 @@ class AccessibilityOverridesPane extends StatelessWidget {
const SizedBox(height: denseSpacing),
_BrightnessOverride(controller: controller),
const SizedBox(height: defaultSpacing),
_TextScaleOverride(controller: controller),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'Bold Text',
description: 'Forces all text in the application to be bold.',
notifier: controller.boldText,
),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'Screen Reader Debugger',
description: 'Debug and test screen reader layouts.',
notifier: controller.screenReader,
),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'High Contrast',
description: 'Increases the contrast of text and icons.',
notifier: controller.highContrast,
),
_ViewMetricsOverrides(controller: controller),
],
),
),
Expand Down Expand Up @@ -128,46 +113,134 @@ class _BrightnessOverride extends StatelessWidget {
}
}

class _TextScaleOverride extends StatelessWidget {
const _TextScaleOverride({required this.controller});
/// The controls of `ext.flutter.viewMetricsOverride`, which are disabled with
/// a tooltip while the app does not support it.
class _ViewMetricsOverrides extends StatelessWidget {
const _ViewMetricsOverrides({required this.controller});

final AccessibilityController controller;

static const _unavailableMessage =
'Unavailable because the connected app is a profile build, or uses a '
'Flutter version that does not support these overrides.';

@override
Widget build(BuildContext context) {
return ValueListenableBuilder<bool>(
valueListenable: controller.overridesAvailable,
builder: (context, available, _) {
final overrides = Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
_TextScaleOverride(controller: controller, enabled: available),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'Bold Text',
description: 'Forces all text in the application to be bold.',
value: controller.boldText,
onChanged: available ? controller.setBoldText : null,
),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'Screen Reader Debugger',
description: 'Debug and test screen reader layouts.',
value: controller.screenReader,
onChanged: available ? controller.setScreenReader : null,
),
const SizedBox(height: defaultSpacing),
_SwitchOverride(
label: 'High Contrast',
description: 'Increases the contrast of text and icons.',
value: controller.highContrast,
onChanged: available ? controller.setHighContrast : null,
),
],
);
if (available) return overrides;
return DevToolsTooltip(message: _unavailableMessage, child: overrides);
},
);
}
}

class _TextScaleOverride extends StatefulWidget {
const _TextScaleOverride({required this.controller, required this.enabled});

final AccessibilityController controller;

/// Whether the text scale can be changed.
final bool enabled;

@override
State<_TextScaleOverride> createState() => _TextScaleOverrideState();
}

class _TextScaleOverrideState extends State<_TextScaleOverride> {
static const _minTextScale = 0.5;
static const _maxTextScale = 3.0;
static const _textScaleDivisions = 25;

/// The text scale that the slider shows while the user moves it, until the
/// app answers the override.
double? _sliderTextScale;

Future<void> _overrideTextScale(double textScale) async {
await widget.controller.setTextScale(textScale);
if (mounted && _sliderTextScale == textScale) {
setState(() => _sliderTextScale = null);
}
}
Comment on lines +187 to +192

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Wrapping the call in a try-finally block ensures that _sliderTextScale is reset to null even if the service call fails. This prevents the slider from getting stuck at an invalid/failed value and allows it to snap back to the actual value reported by the app.

  Future<void> _overrideTextScale(double textScale) async {
    try {
      await widget.controller.setTextScale(textScale);
    } finally {
      if (mounted && _sliderTextScale == textScale) {
        setState(() => _sliderTextScale = null);
      }
    }
  }
References
  1. Prioritize logic, performance on the UI thread, and architectural consistency. (link)


@override
Widget build(BuildContext context) {
final theme = Theme.of(context);
return ValueListenableBuilder<double>(
valueListenable: controller.textScale,
builder: (context, value, _) {
final enabled = widget.enabled;
return ValueListenableBuilder<double?>(
valueListenable: widget.controller.textScale,
builder: (context, appTextScale, _) {
final textScale = _sliderTextScale ?? appTextScale;
return Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: [
const _AccessibilityPanelLabel(
label: 'Text Scale',
description: 'Scale the system font size.',
const Expanded(
child: _AccessibilityPanelLabel(
label: 'Text Scale',
description: 'Scale the system font size.',
),
),
Text(
'${value.toStringAsFixed(2)}x',
// 1.0 is an override too, so the text scale of the platform
// is shown by name rather than as a factor.
textScale == null
? 'System default'
: '${textScale.toStringAsFixed(2)}x',
style: theme.boldTextStyle,
),
const SizedBox(width: denseSpacing),
DevToolsButton.iconOnly(
icon: Icons.restart_alt,
outlined: false,
tooltip: 'Use the system text scale',
onPressed: enabled && appTextScale != null
? () => unawaited(widget.controller.setTextScale(null))
: null,
),
],
),
const SizedBox(height: densePadding),
Slider(
value: value,
value: (textScale ?? 1.0).clamp(_minTextScale, _maxTextScale),
min: _minTextScale,
max: _maxTextScale,
divisions: _textScaleDivisions,
onChanged: (newValue) {
controller.textScale.value = newValue;
},
onChanged: enabled
? (value) => setState(() => _sliderTextScale = value)
: null,
onChangeEnd: enabled
? (value) => unawaited(_overrideTextScale(value))
: null,
),
],
);
Expand All @@ -180,12 +253,18 @@ class _SwitchOverride extends StatelessWidget {
const _SwitchOverride({
required this.label,
required this.description,
required this.notifier,
required this.value,
required this.onChanged,
});

final String label;
final String description;
final ValueNotifier<bool> notifier;

/// Whether the app has this override on.
final ValueListenable<bool> value;

/// Called when the user toggles the switch, or null to disable the switch.
final ValueChanged<bool>? onChanged;

@override
Widget build(BuildContext context) {
Expand All @@ -197,7 +276,11 @@ class _SwitchOverride extends StatelessWidget {
description: description,
),
),
NotifierSwitch(notifier: notifier),
ValueListenableBuilder<bool>(
valueListenable: value,
builder: (context, isOn, _) =>
DevToolsSwitch(value: isOn, onChanged: onChanged),
),
],
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ const getIssuesAndSemanticsTree =

/// Service extension to dispose semantics on the connected app.
const disposeSemantics = 'ext.flutter.accessibility.disposeSemantics';

/// Service extension to override the view metrics of the connected app.
const viewMetricsOverride = 'ext.flutter.viewMetricsOverride';
Loading
Loading