Commit c312b3d9 authored by amirh's avatar amirh Committed by GitHub

Keep Icon's subtree structure the same with and without a semanticLabel. (#12516)

* Keep Icon's subtree structure the same with and without a semanticLabel.

This avoids rebuilding the subtree when a semanticLabel is set/unset.
Also updated some doc comments.

(This is a followup on post-merge comments for #12475)
parent 72c8d9b4
......@@ -89,7 +89,10 @@ class Icon extends StatelessWidget {
/// This would be read out in accessibility modes (e.g TalkBack/VoiceOver).
/// This label does not show in the UI.
///
/// See [Semantics.label];
/// See also:
///
/// * [Semantics.label] which is set with [semanticLabel] in the underlying
/// [Semantics] widget.
final String semanticLabel;
@override
......@@ -102,15 +105,19 @@ class Icon extends StatelessWidget {
final double iconSize = size ?? iconTheme.size;
if (icon == null)
return _wrapWithSemantics(new SizedBox(width: iconSize, height: iconSize));
return new Semantics(
label: semanticLabel,
child: new SizedBox(width: iconSize, height: iconSize)
);
final double iconOpacity = iconTheme.opacity;
Color iconColor = color ?? iconTheme.color;
if (iconOpacity != 1.0)
iconColor = iconColor.withOpacity(iconColor.opacity * iconOpacity);
return _wrapWithSemantics(
new ExcludeSemantics(
return new Semantics(
label: semanticLabel,
child: new ExcludeSemantics(
child: new SizedBox(
width: iconSize,
height: iconSize,
......@@ -133,17 +140,6 @@ class Icon extends StatelessWidget {
);
}
/// Wraps the widget with a Semantics widget if [semanticLabel] is set.
Widget _wrapWithSemantics(Widget widget) {
if (semanticLabel == null)
return widget;
return new Semantics(
child: widget,
label: semanticLabel,
);
}
@override
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
......
......@@ -161,4 +161,35 @@ void main() {
expect(semantics, hasSemantics(new TestSemantics.root(label: 'a label')));
});
testWidgets('Changing semantic label from null doesn\'t rebuild tree ', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: const Center(
child: const Icon(Icons.time_to_leave),
),
),
);
final Element richText1 = tester.element(find.byType(RichText));
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: const Center(
child: const Icon(
Icons.time_to_leave,
semanticLabel: 'a label',
),
),
),
);
final Element richText2 = tester.element(find.byType(RichText));
// Compare a leaf Element in the Icon subtree before and after changing the
// semanticLabel to make sure the subtree was not rebuilt.
expect(richText2, same(richText1));
});
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment