Adding tabSemanticsLabel to CupertinoLocalizations #55336

Piinks · 2020-04-21T21:55:00Z

Description

Closing out this TODO:

flutter/packages/flutter/lib/src/cupertino/bottom_tab_bar.dart

Line 216 in 418007d

    
           // TODO(xster): This needs localization support. https://github.com/flutter/flutter/issues/13452

This localizes the hint for the tab index to CupertinoTabBar.

Related Issues

#13452

Tests

Added unit test for CupertinoLocalizations.tabSemanticsLabel.
Updates affected tests.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

WIP

Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

Piinks · 2020-04-21T21:56:25Z

@shihaohong does this look about right? I'm going to add some tests, just wanted to check that I wired this up right. :)

Piinks · 2020-04-21T21:57:07Z

The script made a very large change to packages/flutter_localizations/lib/src/l10n/cupertino_kn.arb

shihaohong

Everything looks good! The only thing that's missing is running the following commands from your Flutter repo:

dart dev/tools/localization/bin/gen_localizations.dart --overwrite
dart dev/tools/localization/bin/encode_kn_arb_files.dart

The first command generates the localization delegates for each locale so that it will display 'TBD' for non-English locales for the tab label until translations are completed

The second command encodes Kannada script into Unicode escapes so that some versions of emacs do not crash when opening the Flutter repository.

shihaohong · 2020-04-22T16:10:06Z

packages/flutter_localizations/lib/src/l10n/cupertino_kn.arb

-  "copyButtonLabel": "\u0ca8\u0c95\u0cb2\u0cbf\u0cb8\u0cbf",
-  "pasteButtonLabel": "\u0c85\u0c82\u0c9f\u0cbf\u0cb8\u0cbf",
-  "selectAllButtonLabel": "\u0c8e\u0cb2\u0ccd\u0cb2\u0cb5\u0ca8\u0ccd\u0ca8\u0cc2\u0020\u0c86\u0caf\u0ccd\u0c95\u0cc6\u0cae\u0cbe\u0ca1\u0cbf"
+  "datePickerHourSemanticsLabelOne": "$hour ಗಂಟೆ",


This requires running flutter/dev/tools/localization/bin/encode_kn_arb_files.dart. There's a bug with emacs that causes the editor to crash when it detects certain characters in Kannada script.

I need to still make this part of the gen_localizations and gen_date_localizations scripts.

Awesome! Thank you! Updates on the way.

shihaohong · 2020-04-22T17:30:50Z

For a unit test, add something similar to https://github.com/flutter/flutter/blob/master/packages/flutter_localizations/test/cupertino/translations_test.dart#L87 and I think you should be good!

Piinks · 2020-04-22T23:01:03Z

It looks like this may be a breaking change? I think I will have to update the existing tests with CupertinoTabBar to add Localizations. Unless these tests should be getting a default value (like the hard-coded value that was there)

shihaohong · 2020-04-22T23:29:24Z

I think you're right... I just realized that this also changes the behavior for the tab label in other locales (for example, using the es locale would now cause the text to show TBD instead of what we currently have), which is breaking and we should follow the policy for it, but seems acceptable since we've been doing this for every new localized message.

Piinks · 2020-04-23T18:55:35Z

@Hixie and @HansMuller can you advise on this change?

This is breaking in that it

Changes the current hint provided in the CupertinoTabBar for various locales, and
Requires that the use of the CupertinoTabBar will require a Localizations parent to provide the appropriate hint.

@shihaohong mentioned that CupertinoApp should introduce the localizations delegate by default, which would make it less breaking in those cases... although the CupertinoTabBar may not be used in that context alone, as the framework tests prove out.

I'm still diagnosing a Semantics break here as well, just curious in general how this might be made less breaking. A similar change was made for MaterialLocalizations a few years back here: #13714

packages/flutter/lib/src/cupertino/localizations.dart

Hixie · 2020-04-23T19:19:38Z

CupertinoApp should definitely introduce the l10ns, just like MaterialApp does.

Hixie · 2020-04-23T19:20:49Z

i would check with devrel that they agree that having the Cupertino library follow the Material library's lead in how to do l10n is a good idea. assuming that they do, then a migration guide should be simple enough to provide.

shihaohong

LGTM!

packages/flutter/lib/src/cupertino/bottom_tab_bar.dart

packages/flutter/test/cupertino/tab_scaffold_test.dart

Co-Authored-By: Shi-Hao Hong <shihaohong@google.com>

Piinks · 2020-04-23T22:38:46Z

Thanks for the guidance and review @shihaohong! 🎉
I'm putting together a migration guide and gathering some feedback on the break. I'll mark this WIP in the meantime.

…lTabs

packages/flutter/lib/src/cupertino/bottom_tab_bar.dart

xster

Great. Thanks Kate!

xster · 2020-04-29T20:39:39Z

packages/flutter/test/cupertino/bottom_tab_bar_test.dart

+      child: Directionality(
+        textDirection: TextDirection.ltr,
+        child: widget,
+      ),
    ),
  );
 }


can you add one test for the semantics content of the tabs?

There is already a test for that which was checking the value of the hard-coded string, now it is checking the localized result. :)

flutter/packages/flutter/test/cupertino/bottom_tab_bar_test.dart

Line 360 in 0549ab2

testWidgets('tabs announce semantics', (WidgetTester tester) async {

xster · 2020-04-29T20:44:13Z

@Hixie CupertinoApp does introduce localization

…lTabs

codecov · 2020-04-30T22:57:40Z

Codecov Report

Merging #55336 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #55336   +/-   ##
=======================================
  Coverage   70.59%   70.59%           
=======================================
  Files         204      204           
  Lines       22590    22590           
=======================================
  Hits        15947    15947           
  Misses       6643     6643

Flag	Coverage Δ
#flutter_tool	`70.59% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0549ab2...4adb35a. Read the comment docs.

Added tab label o CupertinoLocalizations

3663a41

googlebot added the cla: yes label Apr 21, 2020

fluttergithubbot added the work in progress; do not review label Apr 21, 2020

Piinks mentioned this pull request Apr 21, 2020

ToDo Audit - Cupertino+ Library #54798

Merged

9 tasks

shihaohong reviewed Apr 22, 2020

View reviewed changes

Updated translations, test

30f9f8b

Piinks marked this pull request as ready for review April 22, 2020 22:44

Piinks changed the title ~~[WIP] Adding tabLabel to CupertinoLocalizations~~ Adding tabLabel to CupertinoLocalizations Apr 22, 2020

Piinks removed the work in progress; do not review label Apr 22, 2020

Piinks added the c: API break Backwards-incompatible API changes label Apr 23, 2020

Piinks added 2 commits April 23, 2020 11:31

Update tests

d00eead

Tidying

ab44a3b

Hixie reviewed Apr 23, 2020

View reviewed changes

packages/flutter/lib/src/cupertino/localizations.dart Outdated Show resolved Hide resolved

Piinks added 2 commits April 23, 2020 13:14

Update scaffold test

869c035

Fix semantics

8ad68a4

Piinks changed the title ~~Adding tabLabel to CupertinoLocalizations~~ Adding tabSemanticsLabel to CupertinoLocalizations Apr 23, 2020

Piinks added the a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) label Apr 23, 2020

Refactor to tabSemanticsLabel

43e32a4

shihaohong approved these changes Apr 23, 2020

View reviewed changes

packages/flutter/lib/src/cupertino/bottom_tab_bar.dart Outdated Show resolved Hide resolved

packages/flutter/test/cupertino/tab_scaffold_test.dart Outdated Show resolved Hide resolved

Apply suggestions from code review

a1dab72

Co-Authored-By: Shi-Hao Hong <shihaohong@google.com>

Piinks added the work in progress; do not review label Apr 23, 2020

Piinks added 2 commits April 24, 2020 11:42

Added assertion on localizations

29a7b5a

Merge branch 'cupLocalTabs' of github.com:Piinks/flutter into cupLoca…

84e7de8

…lTabs

Piinks commented Apr 24, 2020

View reviewed changes

packages/flutter/lib/src/cupertino/bottom_tab_bar.dart Outdated Show resolved Hide resolved

Update packages/flutter/lib/src/cupertino/bottom_tab_bar.dart

866aa3e

Piinks commented Apr 24, 2020

View reviewed changes

packages/flutter/lib/src/cupertino/bottom_tab_bar.dart Outdated Show resolved Hide resolved

Update packages/flutter/lib/src/cupertino/bottom_tab_bar.dart

33a86c4

Piinks mentioned this pull request Apr 24, 2020

Migrations guide for Localized CupertinoTabBar Semantics flutter/website#3996

Merged

xster reviewed Apr 29, 2020

View reviewed changes

Piinks added 3 commits April 30, 2020 15:45

Merge branch 'master' of github.com:flutter/flutter into cupLocalTabs

e26b0de

Merge branch 'cupLocalTabs' of github.com:Piinks/flutter into cupLoca…

72d826f

…lTabs

Updated kn

4adb35a

Piinks mentioned this pull request Apr 30, 2020

Step 1 of 3: Add opt-in for debugCheckHasMaterialLocalizations assertion on TextField #56090

Merged

12 tasks

Piinks added waiting for tree to go green and removed work in progress; do not review labels May 1, 2020

fluttergithubbot merged commit 487954a into flutter:master May 1, 2020

Piinks mentioned this pull request Jun 5, 2020

☂️ Technical Debt Audits - Skips & Todos #58841

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding tabSemanticsLabel to CupertinoLocalizations #55336

Adding tabSemanticsLabel to CupertinoLocalizations #55336

Piinks commented Apr 21, 2020 •

edited

Piinks commented Apr 21, 2020

Piinks commented Apr 21, 2020

shihaohong left a comment

shihaohong Apr 22, 2020 •

edited

Piinks Apr 22, 2020

shihaohong commented Apr 22, 2020

Piinks commented Apr 22, 2020

shihaohong commented Apr 22, 2020 •

edited

Piinks commented Apr 23, 2020

Hixie commented Apr 23, 2020

Hixie commented Apr 23, 2020

shihaohong left a comment

Piinks commented Apr 23, 2020

xster left a comment

xster Apr 29, 2020

Piinks Apr 30, 2020

xster commented Apr 29, 2020

codecov bot commented Apr 30, 2020

Adding tabSemanticsLabel to CupertinoLocalizations #55336

Adding tabSemanticsLabel to CupertinoLocalizations #55336

Conversation

Piinks commented Apr 21, 2020 • edited

Description

Related Issues

Tests

Checklist

Breaking Change

Piinks commented Apr 21, 2020

Piinks commented Apr 21, 2020

shihaohong left a comment

Choose a reason for hiding this comment

shihaohong Apr 22, 2020 • edited

Choose a reason for hiding this comment

Piinks Apr 22, 2020

Choose a reason for hiding this comment

shihaohong commented Apr 22, 2020

Piinks commented Apr 22, 2020

shihaohong commented Apr 22, 2020 • edited

Piinks commented Apr 23, 2020

Hixie commented Apr 23, 2020

Hixie commented Apr 23, 2020

shihaohong left a comment

Choose a reason for hiding this comment

Piinks commented Apr 23, 2020

xster left a comment

Choose a reason for hiding this comment

xster Apr 29, 2020

Choose a reason for hiding this comment

Piinks Apr 30, 2020

Choose a reason for hiding this comment

xster commented Apr 29, 2020

codecov bot commented Apr 30, 2020

Codecov Report

Piinks commented Apr 21, 2020 •

edited

shihaohong Apr 22, 2020 •

edited

shihaohong commented Apr 22, 2020 •

edited