From 691cbee61343977c5fe09eef45694f0d755d2d1b Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Sat, 9 Jun 2018 08:23:59 -0700 Subject: [PATCH] Add locale parameter to EditableText (#18222) --- bin/internal/goldens.version | 2 +- .../lib/src/painting/text_painter.dart | 10 +- .../flutter/lib/src/painting/text_style.dart | 7 + .../flutter/lib/src/rendering/editable.dart | 21 +++ .../flutter/lib/src/rendering/paragraph.dart | 1 + .../lib/src/widgets/editable_text.dart | 17 ++ packages/flutter/lib/src/widgets/text.dart | 13 ++ .../flutter/test/material/theme_test.dart | 6 +- .../flutter/test/rendering/editable_test.dart | 4 +- .../test/rendering/localized_fonts_test.dart | 159 ++++++++++++++++++ .../test/rendering/paragraph_test.dart | 2 + 11 files changed, 232 insertions(+), 10 deletions(-) create mode 100644 packages/flutter/test/rendering/localized_fonts_test.dart diff --git a/bin/internal/goldens.version b/bin/internal/goldens.version index 6e25562a8a5..120095b1ed6 100644 --- a/bin/internal/goldens.version +++ b/bin/internal/goldens.version @@ -1 +1 @@ -760320e3fc8ccd12deb4066bd4033a98d078359f +568342373b14fab81fd665d397c8c00db3d46fca diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index 4704b34f0d8..3da44f1cc18 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:ui' as ui show Paragraph, ParagraphBuilder, ParagraphConstraints, ParagraphStyle, Locale; +import 'dart:ui' as ui show Paragraph, ParagraphBuilder, ParagraphConstraints, ParagraphStyle; import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; @@ -46,7 +46,7 @@ class TextPainter { double textScaleFactor = 1.0, int maxLines, String ellipsis, - ui.Locale locale, + Locale locale, }) : assert(text == null || text.debugAssertIsValid()), assert(textAlign != null), assert(textScaleFactor != null), @@ -168,9 +168,9 @@ class TextPainter { } /// The locale used to select region-specific glyphs. - ui.Locale get locale => _locale; - ui.Locale _locale; - set locale(ui.Locale value) { + Locale get locale => _locale; + Locale _locale; + set locale(Locale value) { if (_locale == value) return; _locale = value; diff --git a/packages/flutter/lib/src/painting/text_style.dart b/packages/flutter/lib/src/painting/text_style.dart index 36b7f125c01..36b4a6d7e5a 100644 --- a/packages/flutter/lib/src/painting/text_style.dart +++ b/packages/flutter/lib/src/painting/text_style.dart @@ -299,6 +299,13 @@ class TextStyle extends Diagnosticable { final double height; /// The locale used to select region-specific glyphs. + /// + /// This property is rarely set. Typically the locale used to select + /// region-specific glyphs is defined by the text widget's [BuildContext] + /// using `Localizations.localeOf(context)`. For example [RichText] defines + /// its locale this way. However, a rich text widget's [TextSpan]s could specify + /// text styles with different explicit locales in order to select different + /// region-specifc glyphs for each text span. final Locale locale; /// The paint drawn as a background for the text. diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 6052fed969a..40903409c22 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -133,6 +133,7 @@ class RenderEditable extends RenderBox { this.onCaretChanged, this.ignorePointer = false, bool obscureText = false, + Locale locale, }) : assert(textAlign != null), assert(textDirection != null, 'RenderEditable created without a textDirection.'), assert(maxLines == null || maxLines > 0), @@ -145,6 +146,7 @@ class RenderEditable extends RenderBox { textAlign: textAlign, textDirection: textDirection, textScaleFactor: textScaleFactor, + locale: locale, ), _cursorColor = cursorColor, _showCursor = showCursor ?? new ValueNotifier(false), @@ -249,6 +251,24 @@ class RenderEditable extends RenderBox { markNeedsSemanticsUpdate(); } + /// Used by this renderer's internal [TextPainter] to select a locale-specific + /// font. + /// + /// In some cases the same Unicode character may be rendered differently depending + /// on the locale. For example the '骨' character is rendered differently in + /// the Chinese and Japanese locales. In these cases the [locale] may be used + /// to select a locale-specific font. + /// + /// If this value is null, a system-dependent algorithm is used to select + /// the font. + Locale get locale => _textPainter.locale; + set locale(Locale value) { + if (_textPainter.locale == value) + return; + _textPainter.locale = value; + markNeedsTextLayout(); + } + /// The color to use when painting the cursor. Color get cursorColor => _cursorColor; Color _cursorColor; @@ -749,6 +769,7 @@ class RenderEditable extends RenderBox { properties.add(new IntProperty('maxLines', maxLines)); properties.add(new DiagnosticsProperty('selectionColor', selectionColor)); properties.add(new DoubleProperty('textScaleFactor', textScaleFactor)); + properties.add(new DiagnosticsProperty('locale', locale, defaultValue: null)); properties.add(new DiagnosticsProperty('selection', selection)); properties.add(new DiagnosticsProperty('offset', offset)); } diff --git a/packages/flutter/lib/src/rendering/paragraph.dart b/packages/flutter/lib/src/rendering/paragraph.dart index 7f813a68a3b..31f7b8b5b3e 100644 --- a/packages/flutter/lib/src/rendering/paragraph.dart +++ b/packages/flutter/lib/src/rendering/paragraph.dart @@ -458,6 +458,7 @@ class RenderParagraph extends RenderBox { properties.add(new FlagProperty('softWrap', value: softWrap, ifTrue: 'wrapping at box width', ifFalse: 'no wrapping except at line break characters', showName: true)); properties.add(new EnumProperty('overflow', overflow)); properties.add(new DoubleProperty('textScaleFactor', textScaleFactor, defaultValue: 1.0)); + properties.add(new DiagnosticsProperty('locale', locale, defaultValue: null)); properties.add(new IntProperty('maxLines', maxLines, ifNull: 'unlimited')); } } diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index e9ff2c7f2ea..b7664dc258a 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -12,6 +12,7 @@ import 'basic.dart'; import 'focus_manager.dart'; import 'focus_scope.dart'; import 'framework.dart'; +import 'localizations.dart'; import 'media_query.dart'; import 'scroll_controller.dart'; import 'scroll_physics.dart'; @@ -160,6 +161,7 @@ class EditableText extends StatefulWidget { @required this.cursorColor, this.textAlign = TextAlign.start, this.textDirection, + this.locale, this.textScaleFactor, this.maxLines = 1, this.autofocus = false, @@ -229,6 +231,15 @@ class EditableText extends StatefulWidget { /// Defaults to the ambient [Directionality], if any. final TextDirection textDirection; + /// Used to select a font when the same Unicode character can + /// be rendered differently, depending on the locale. + /// + /// It's rarely necessary to set this property. By default its value + /// is inherited from the enclosing app with `Localizations.localeOf(context)`. + /// + /// See [RenderEditable.locale] for more information. + final Locale locale; + /// The number of font pixels for each logical pixel. /// /// For example, if the text scale factor is 1.5, text will be 50% larger than @@ -299,6 +310,7 @@ class EditableText extends StatefulWidget { style?.debugFillProperties(properties); properties.add(new EnumProperty('textAlign', textAlign, defaultValue: null)); properties.add(new EnumProperty('textDirection', textDirection, defaultValue: null)); + properties.add(new DiagnosticsProperty('locale', locale, defaultValue: null)); properties.add(new DoubleProperty('textScaleFactor', textScaleFactor, defaultValue: null)); properties.add(new IntProperty('maxLines', maxLines, defaultValue: 1)); properties.add(new DiagnosticsProperty('autofocus', autofocus, defaultValue: false)); @@ -687,6 +699,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien textScaleFactor: widget.textScaleFactor ?? MediaQuery.textScaleFactorOf(context), textAlign: widget.textAlign, textDirection: _textDirection, + locale: widget.locale, obscureText: widget.obscureText, autocorrect: widget.autocorrect, offset: offset, @@ -747,6 +760,7 @@ class _Editable extends LeafRenderObjectWidget { this.textScaleFactor, this.textAlign, @required this.textDirection, + this.locale, this.obscureText, this.autocorrect, this.offset, @@ -767,6 +781,7 @@ class _Editable extends LeafRenderObjectWidget { final double textScaleFactor; final TextAlign textAlign; final TextDirection textDirection; + final Locale locale; final bool obscureText; final bool autocorrect; final ViewportOffset offset; @@ -786,6 +801,7 @@ class _Editable extends LeafRenderObjectWidget { textScaleFactor: textScaleFactor, textAlign: textAlign, textDirection: textDirection, + locale: locale ?? Localizations.localeOf(context, nullOk: true), selection: value.selection, offset: offset, onSelectionChanged: onSelectionChanged, @@ -807,6 +823,7 @@ class _Editable extends LeafRenderObjectWidget { ..textScaleFactor = textScaleFactor ..textAlign = textAlign ..textDirection = textDirection + ..locale = locale ?? Localizations.localeOf(context, nullOk: true) ..selection = value.selection ..offset = offset ..onSelectionChanged = onSelectionChanged diff --git a/packages/flutter/lib/src/widgets/text.dart b/packages/flutter/lib/src/widgets/text.dart index bb7e85b8dc3..c88e8be956f 100644 --- a/packages/flutter/lib/src/widgets/text.dart +++ b/packages/flutter/lib/src/widgets/text.dart @@ -206,6 +206,7 @@ class Text extends StatelessWidget { this.style, this.textAlign, this.textDirection, + this.locale, this.softWrap, this.overflow, this.textScaleFactor, @@ -220,6 +221,7 @@ class Text extends StatelessWidget { this.style, this.textAlign, this.textDirection, + this.locale, this.softWrap, this.overflow, this.textScaleFactor, @@ -263,6 +265,15 @@ class Text extends StatelessWidget { /// Defaults to the ambient [Directionality], if any. final TextDirection textDirection; + /// Used to select a font when the same Unicode character can + /// be rendered differently, depending on the locale. + /// + /// It's rarely necessary to set this property. By default its value + /// is inherited from the enclosing app with `Localizations.localeOf(context)`. + /// + /// See [RenderParagraph.locale] for more information. + final Locale locale; + /// Whether the text should break at soft line breaks. /// /// If false, the glyphs in the text will be positioned as if there was unlimited horizontal space. @@ -303,6 +314,7 @@ class Text extends StatelessWidget { return new RichText( textAlign: textAlign ?? defaultTextStyle.textAlign ?? TextAlign.start, textDirection: textDirection, // RichText uses Directionality.of to obtain a default if this is null. + locale: locale, // RichText uses Localizations.localeOf to obtain a default if this is null softWrap: softWrap ?? defaultTextStyle.softWrap, overflow: overflow ?? defaultTextStyle.overflow, textScaleFactor: textScaleFactor ?? MediaQuery.textScaleFactorOf(context), @@ -325,6 +337,7 @@ class Text extends StatelessWidget { style?.debugFillProperties(properties); properties.add(new EnumProperty('textAlign', textAlign, defaultValue: null)); properties.add(new EnumProperty('textDirection', textDirection, defaultValue: null)); + properties.add(new DiagnosticsProperty('locale', locale, defaultValue: null)); properties.add(new FlagProperty('softWrap', value: softWrap, ifTrue: 'wrapping at box width', ifFalse: 'no wrapping except at line break characters', showName: true)); properties.add(new EnumProperty('overflow', overflow, defaultValue: null)); properties.add(new DoubleProperty('textScaleFactor', textScaleFactor, defaultValue: null)); diff --git a/packages/flutter/test/material/theme_test.dart b/packages/flutter/test/material/theme_test.dart index ec029141528..e8db419d315 100644 --- a/packages/flutter/test/material/theme_test.dart +++ b/packages/flutter/test/material/theme_test.dart @@ -451,7 +451,7 @@ class _TextStyleProxy implements TextStyle { @override FontStyle get fontStyle => _delegate.fontStyle; @override FontWeight get fontWeight => _delegate.fontWeight; @override double get height => _delegate.height; - @override ui.Locale get locale => _delegate.locale; + @override Locale get locale => _delegate.locale; @override ui.Paint get background => _delegate.background; @override bool get inherit => _delegate.inherit; @override double get letterSpacing => _delegate.letterSpacing; @@ -479,7 +479,7 @@ class _TextStyleProxy implements TextStyle { } @override - TextStyle copyWith({Color color, String fontFamily, double fontSize, FontWeight fontWeight, FontStyle fontStyle, double letterSpacing, double wordSpacing, TextBaseline textBaseline, double height, ui.Locale locale, ui.Paint background, TextDecoration decoration, Color decorationColor, TextDecorationStyle decorationStyle, String debugLabel}) { + TextStyle copyWith({Color color, String fontFamily, double fontSize, FontWeight fontWeight, FontStyle fontStyle, double letterSpacing, double wordSpacing, TextBaseline textBaseline, double height, Locale locale, ui.Paint background, TextDecoration decoration, Color decorationColor, TextDecorationStyle decorationStyle, String debugLabel}) { throw new UnimplementedError(); } @@ -489,7 +489,7 @@ class _TextStyleProxy implements TextStyle { } @override - ui.ParagraphStyle getParagraphStyle({TextAlign textAlign, TextDirection textDirection, double textScaleFactor = 1.0, String ellipsis, int maxLines, ui.Locale locale}) { + ui.ParagraphStyle getParagraphStyle({TextAlign textAlign, TextDirection textDirection, double textScaleFactor = 1.0, String ellipsis, int maxLines, Locale locale}) { throw new UnimplementedError(); } diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart index 5f0d8ca6298..47c8ab35cf6 100644 --- a/packages/flutter/test/rendering/editable_test.dart +++ b/packages/flutter/test/rendering/editable_test.dart @@ -14,6 +14,7 @@ void main() { ), textAlign: TextAlign.start, textDirection: TextDirection.ltr, + locale: const Locale('ja', 'JP'), offset: new ViewportOffset.zero(), ); expect(editable.getMinIntrinsicWidth(double.infinity), 50.0); @@ -33,6 +34,7 @@ void main() { ' │ maxLines: 1\n' ' │ selectionColor: null\n' ' │ textScaleFactor: 1.0\n' + ' │ locale: ja_JP\n' ' │ selection: null\n' ' │ offset: _FixedViewportOffset#00000(offset: 0.0)\n' ' ╘═╦══ text ═══\n' @@ -46,4 +48,4 @@ void main() { ), ); }); -} \ No newline at end of file +} diff --git a/packages/flutter/test/rendering/localized_fonts_test.dart b/packages/flutter/test/rendering/localized_fonts_test.dart new file mode 100644 index 00000000000..ed18ab23257 --- /dev/null +++ b/packages/flutter/test/rendering/localized_fonts_test.dart @@ -0,0 +1,159 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io' show Platform; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +// TODO(hansmuller): when https://github.com/flutter/flutter/issues/17700 +// is fixed, these tests should be updated to use a real font (not Ahem). + +void main() { + testWidgets( + 'RichText TextSpan styles with different locales', + (WidgetTester tester) async { + + await tester.pumpWidget( + new MaterialApp( + supportedLocales: const [ + const Locale('en', 'US'), + const Locale('ja'), + const Locale('zh'), + ], + home: new Builder( + builder: (BuildContext context) { + const String character = '骨'; + final TextStyle style = Theme.of(context).textTheme.display3; + return new Scaffold( + body: new Container( + padding: const EdgeInsets.all(48.0), + alignment: Alignment.center, + child: new RepaintBoundary( + // Expected result can be seen here: + // https://user-images.githubusercontent.com/1377460/40503473-faad6f34-5f42-11e8-972b-d83b727c9d0e.png + child: new RichText( + text: new TextSpan( + children: [ + new TextSpan(text: character, style: style.copyWith(locale: const Locale('ja'))), + new TextSpan(text: character, style: style.copyWith(locale: const Locale('zh'))), + ], + ), + ), + ), + ), + ); + }, + ), + ) + ); + + await expectLater( + find.byType(RichText), + matchesGoldenFile('localized_fonts.rich_text.styled_text_span.png'), + skip: !Platform.isLinux, + ); + }, + skip: !Platform.isLinux, + ); + + testWidgets( + 'Text with locale-specific glyphs, ambient locale', + (WidgetTester tester) async { + await tester.pumpWidget( + new MaterialApp( + supportedLocales: const [ + const Locale('en', 'US'), + const Locale('ja'), + const Locale('zh'), + ], + home: new Builder( + builder: (BuildContext context) { + const String character = '骨'; + final TextStyle style = Theme.of(context).textTheme.display3; + return new Scaffold( + body: new Container( + padding: const EdgeInsets.all(48.0), + alignment: Alignment.center, + child: new RepaintBoundary( + // Expected result can be seen here: + // https://user-images.githubusercontent.com/1377460/40503473-faad6f34-5f42-11e8-972b-d83b727c9d0e.png + child: new Row( + mainAxisAlignment: MainAxisAlignment.spaceEvenly, + children: [ + new Localizations.override( + context: context, + locale: const Locale('ja'), + child: new Text(character, style: style), + ), + new Localizations.override( + context: context, + locale: const Locale('zh'), + child: new Text(character, style: style), + ), + ], + ), + ), + ), + ); + }, + ), + ) + ); + + await expectLater( + find.byType(Row), + matchesGoldenFile('localized_fonts.text_ambient_locale.chars.png'), + skip: !Platform.isLinux, + ); + }, + skip: !Platform.isLinux, + ); + + testWidgets( + 'Text with locale-specific glyphs, explicit locale', + (WidgetTester tester) async { + await tester.pumpWidget( + new MaterialApp( + supportedLocales: const [ + const Locale('en', 'US'), + const Locale('ja'), + const Locale('zh'), + ], + home: new Builder( + builder: (BuildContext context) { + const String character = '骨'; + final TextStyle style = Theme.of(context).textTheme.display3; + return new Scaffold( + body: new Container( + padding: const EdgeInsets.all(48.0), + alignment: Alignment.center, + child: new RepaintBoundary( + // Expected result can be seen here: + // https://user-images.githubusercontent.com/1377460/40503473-faad6f34-5f42-11e8-972b-d83b727c9d0e.png + child: new Row( + mainAxisAlignment: MainAxisAlignment.spaceEvenly, + children: [ + new Text(character, style: style, locale: const Locale('ja')), + new Text(character, style: style, locale: const Locale('zh')), + ], + ), + ), + ), + ); + }, + ), + ) + ); + + await expectLater( + find.byType(Row), + matchesGoldenFile('localized_fonts.text_explicit_locale.chars.png'), + skip: !Platform.isLinux, + ); + }, + skip: !Platform.isLinux, + ); + +} diff --git a/packages/flutter/test/rendering/paragraph_test.dart b/packages/flutter/test/rendering/paragraph_test.dart index ca8bd3f0ef7..7130ef30a4b 100644 --- a/packages/flutter/test/rendering/paragraph_test.dart +++ b/packages/flutter/test/rendering/paragraph_test.dart @@ -288,6 +288,7 @@ void main() { final RenderParagraph paragraph = new RenderParagraph( const TextSpan(text: _kText), textDirection: TextDirection.ltr, + locale: const Locale('ja', 'JP'), ); expect(paragraph, hasAGoodToStringDeep); expect( @@ -301,6 +302,7 @@ void main() { ' │ textDirection: ltr\n' ' │ softWrap: wrapping at box width\n' ' │ overflow: clip\n' + ' │ locale: ja_JP\n' ' │ maxLines: unlimited\n' ' ╘═╦══ text ═══\n' ' ║ TextSpan:\n'