fixes l10n for CupertinoDatePicker in monthYear mode #130934

AndreySuworow · 2023-07-19T20:00:10Z

This PR fixes l10n issue when months names are being used in incorrect form in CupertinoDatePicker in CupertinoDatePickerMode.yearMonth (#130930).

The idea of this proposal is to add an optional parameter standalone for CupertinoLocalizations.datePickerMonth to be able to choose when to use months names in base form (intl DateSymbols.STANDALONEMONTHS) and when in day-dependent form (intl DateSymbols.MONTHS)

Before

After

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

flutter-dashboard · 2023-07-19T20:00:12Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

MitchellGoodwin · 2023-07-20T21:16:53Z

Thank you for your contribution! This would be a great change to pull in.

There are a few issues that the Linux analyze test found with formatting. Also whenever possible we try not to add parameters to classes unnecessarily. I think we can safely assume that the base form would only be used in yearMonth for example, so adding the customizability would be bloat. Similarly, it might be better to not add another parameter to CupertinoLocalizations.datePickerMonth and maybe instead make another method for CupertinoLocalizations (maybe CupertinoLocalizations.standaloneMonth?) that can be conditionally used instead by the date picker. That looks closer to how Material does it, and seems a bit cleaner.

AndreySuworow · 2023-07-24T19:46:22Z

@MitchellGoodwin thanks for useful tips

I've made some improvements according to your suggestions. Also I've decided to add capitalization feature to make picker behavior more native-like:

Details

Native iOS	Before	After

MitchellGoodwin

Thank you for making the changes. The linter is complaining about some formatting in the tests:

   info • Use 'const' literals as arguments to constructors of '@immutable' classes • packages/flutter_localizations/test/cupertino/date_picker_test.dart:21:27 • prefer_const_literals_to_create_immutables
   info • Use 'const' with the constructor to improve performance • packages/flutter_localizations/test/cupertino/date_picker_test.dart:21:28 • prefer_const_constructors
   info • Missing type annotation • packages/flutter_localizations/test/cupertino/date_picker_test.dart:22:33 • always_specify_types
   info • Use 'const' literals as arguments to constructors of '@immutable' classes • packages/flutter_localizations/test/cupertino/date_picker_test.dart:22:33 • prefer_const_literals_to_create_immutables
   info • Missing type annotation • packages/flutter_localizations/test/cupertino/date_picker_test.dart:40:27 • always_specify_types
   info • Use 'const' literals as arguments to constructors of '@immutable' classes • packages/flutter_localizations/test/cupertino/date_picker_test.dart:40:27 • prefer_const_literals_to_create_immutables
   info • Use 'const' with the constructor to improve performance • packages/flutter_localizations/test/cupertino/date_picker_test.dart:40:28 • prefer_const_constructors
   info • Missing type annotation • packages/flutter_localizations/test/cupertino/date_picker_test.dart:41:33 • always_specify_types
   info • Use 'const' literals as arguments to constructors of '@immutable' classes • packages/flutter_localizations/test/cupertino/date_picker_test.dart:41:33 • prefer_const_literals_to_create_immutables```

MitchellGoodwin · 2023-07-24T22:59:35Z

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

@@ -438,7 +438,8 @@ class CupertinoDatePicker extends StatefulWidget {
    _PickerColumnType columnType,
    CupertinoLocalizations localizations,
    BuildContext context,
-    bool showDayOfWeek
+    bool showDayOfWeek,
+    [bool standaloneMonth = false]


I think this would be better as a non-positional parameter. We tend to not use these when there are multiple parameters. It should be left to default to false, though.

MitchellGoodwin

LGTM! Thank you for putting this together.

justinmc

LGTM with nits 👍

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

packages/flutter_localizations/lib/src/cupertino_localizations.dart

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

justinmc

Thanks for fixing my nits!

AndreySuworow · 2023-08-16T22:22:50Z

@MitchellGoodwin, can someone merge this PR, since I don't have access. Or should this PR be reviewed by someone else?

MitchellGoodwin · 2023-08-17T17:09:53Z

@AndreySuworow apologies, it looks like it got stuck in Google testing, and it needs to be rebased. Would you mind doing that when you have a chance? Getting all the checks green, besides tree status, is that last step before it's ready to be merged.

@justinmc

according to @justinmc suggestions

AndreySuworow · 2023-08-18T10:59:59Z

@MitchellGoodwin
✅ Done. Everything seems to be OK now

MitchellGoodwin · 2023-08-18T16:39:02Z

@AndreySuworow Thank you! The bot should take it from here. Thank you again for the PR.

…er/flutter#130934)

flutter/flutter@5a556f8...6f227c0 2023-08-19 jacksongardner@google.com Space character should be optional when tree shaking fonts (flutter/flutter#132880) 2023-08-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 654d3a2c8494 to f4bffdcf8536 (5 revisions) (flutter/flutter#132876) 2023-08-18 rmolivares@renzo-olivares.dev SelectionArea on iOS should toggle the context menu when tapping on the previous selection (flutter/flutter#132851) 2023-08-18 zanderso@users.noreply.github.com Forward port API docs creation realm handling from prior script (flutter/flutter#132867) 2023-08-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 72c4e61fdbfa to 654d3a2c8494 (3 revisions) (flutter/flutter#132868) 2023-08-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 83fdadaf12f0 to 72c4e61fdbfa (2 revisions) (flutter/flutter#132861) 2023-08-18 47866232+chunhtai@users.noreply.github.com Updates app link gradle tasks and remove vm services (flutter/flutter#131805) 2023-08-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 58f7d8ee3e2c to 83fdadaf12f0 (7 revisions) (flutter/flutter#132857) 2023-08-18 mustafauzun0@gmail.com doc: add flag params (flutter/flutter#132485) 2023-08-18 139406084+gmilou@users.noreply.github.com Add a new MatrixTransition and refactor ScaleTransition and RotationTâ�¦ (flutter/flutter#131084) 2023-08-18 32621121+AndreySuworow@users.noreply.github.com fixes l10n for CupertinoDatePicker in monthYear mode (flutter/flutter#130934) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…er/flutter#130934)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: internationalization Supporting other languages or locales. (aka i18n) f: cupertino flutter/packages/flutter/cupertino repository labels Jul 19, 2023

MitchellGoodwin reviewed Jul 24, 2023

View reviewed changes

MitchellGoodwin mentioned this pull request Jul 28, 2023

Incorrect l10n for CupertinoDatePicker in monthYear mode #130930

Closed

2 tasks

MitchellGoodwin approved these changes Aug 1, 2023

View reviewed changes

cbracken approved these changes Aug 1, 2023

View reviewed changes

justinmc approved these changes Aug 1, 2023

View reviewed changes

justinmc approved these changes Aug 7, 2023

View reviewed changes

AndreySuworow added 6 commits August 18, 2023 04:50

fixes l10n for CupertinoDatePicker in monthYear mode

5cb67ad

add test for new param in l10n

1b17e34

Add CupertinoDatePicker tests

e685cf1

move standalone to separate method

2fdda6c

style & linter fixes

dbbaec5

style fixes

b182284

according to @justinmc suggestions

AndreySuworow force-pushed the datepicker-months-l10n branch from 19e971a to b182284 Compare August 18, 2023 01:50

MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 18, 2023

auto-submit bot merged commit 1cfba26 into flutter:master Aug 18, 2023
73 checks passed

AndreySuworow deleted the datepicker-months-l10n branch August 18, 2023 18:02

engine-flutter-autoroll mentioned this pull request Aug 19, 2023

Roll Flutter from 5a556f8ecde6 to 6f227c078430 (11 revisions) flutter/packages#4748

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 19, 2023

1cfba2620 fixes l10n for CupertinoDatePicker in monthYear mode (flutt…

56325ec

…er/flutter#130934)

engine-flutter-autoroll mentioned this pull request Aug 19, 2023

Roll Flutter from 5a556f8ecde6 to 6f227c078430 (11 revisions) flutter/packages#4749

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 19, 2023

1cfba2620 fixes l10n for CupertinoDatePicker in monthYear mode (flutt…

d9ff15d

…er/flutter#130934)

HansMuller mentioned this pull request Aug 25, 2023

Flutter Framework Team: Notable Commits #121415

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023

1cfba2620 fixes l10n for CupertinoDatePicker in monthYear mode (flutt…

ce5fd6f

…er/flutter#130934)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes l10n for CupertinoDatePicker in monthYear mode #130934

fixes l10n for CupertinoDatePicker in monthYear mode #130934

AndreySuworow commented Jul 19, 2023 •

edited

flutter-dashboard bot commented Jul 19, 2023

MitchellGoodwin commented Jul 20, 2023

AndreySuworow commented Jul 24, 2023

MitchellGoodwin left a comment

MitchellGoodwin Jul 24, 2023

AndreySuworow Jul 25, 2023

MitchellGoodwin left a comment

justinmc left a comment

justinmc left a comment

AndreySuworow commented Aug 16, 2023

MitchellGoodwin commented Aug 17, 2023

AndreySuworow commented Aug 18, 2023 •

edited

MitchellGoodwin commented Aug 18, 2023

fixes l10n for CupertinoDatePicker in monthYear mode #130934

fixes l10n for CupertinoDatePicker in monthYear mode #130934

Conversation

AndreySuworow commented Jul 19, 2023 • edited

Pre-launch Checklist

flutter-dashboard bot commented Jul 19, 2023

MitchellGoodwin commented Jul 20, 2023

AndreySuworow commented Jul 24, 2023

MitchellGoodwin left a comment

Choose a reason for hiding this comment

MitchellGoodwin Jul 24, 2023

Choose a reason for hiding this comment

AndreySuworow Jul 25, 2023

Choose a reason for hiding this comment

MitchellGoodwin left a comment

Choose a reason for hiding this comment

justinmc left a comment

Choose a reason for hiding this comment

justinmc left a comment

Choose a reason for hiding this comment

AndreySuworow commented Aug 16, 2023

MitchellGoodwin commented Aug 17, 2023

AndreySuworow commented Aug 18, 2023 • edited

MitchellGoodwin commented Aug 18, 2023

AndreySuworow commented Jul 19, 2023 •

edited

AndreySuworow commented Aug 18, 2023 •

edited