Add rfw widgets #5661

peixinli · 2023-12-13T06:15:22Z

Add two rfw widgets(ClipRRect and DropdownButton) and flutter format the widget files.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2023-12-13T06:15:24Z

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 "@text-exemption-reviewers" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use 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.

Hixie · 2023-12-16T20:06:19Z

packages/rfw/lib/src/flutter/core_widgets.dart


 // In these widgets we make an effort to expose every single argument available.
-Map<String, LocalWidgetBuilder> get _coreWidgetsDefinitions => <String, LocalWidgetBuilder>{


can you keep the original formatting? it's much, much easier to review changes with the old formatting than with what dart format does.

Reverted. Should I format it in another pr or it's ok to just keep it what it is?

Hixie · 2023-12-16T20:06:44Z

This will need tests. In particular, this package requires 100% test coverage.

peixinli · 2023-12-26T23:50:14Z

This will need tests. In particular, this package requires 100% test coverage.

Done! I updated the golden files as well. It seems disabled now though.

packages/rfw/lib/src/flutter/core_widgets.dart

packages/rfw/lib/src/flutter/material_widgets.dart

Hixie · 2023-12-27T04:43:15Z

packages/rfw/lib/src/flutter/material_widgets.dart

+    final length = source.length(['items']);
+    final dropdownMenuItems = List<DropdownMenuItem<Object>>.generate(
+        length,
+        (i) => DropdownMenuItem<Object>(


might be clearer if i was called index or some such
also, missing type ((int index))

packages/rfw/lib/src/flutter/material_widgets.dart

Hixie · 2023-12-27T04:46:09Z

packages/rfw/lib/src/flutter/material_widgets.dart

+///  * [DropdownButton] takes a list of items object with the key of params 
+///    names and value of the param values to represent [DropdownMenuItem].


I'm not sure exactly what this is trying to say but it doesn't seem to match the code — it seems to be a list of objects, and in each object there's zero or more of onTap, value, enabled, alignment, and child? (Not sure which are required and which are optional, but we should document that too.)

Updated the comment.

Hixie · 2023-12-27T04:47:27Z

packages/rfw/lib/src/flutter/material_widgets.dart

+        length,
+        (i) => DropdownMenuItem<Object>(
+            onTap: source.voidHandler(['items', i, 'onTap']),
+            value: source.v<String>(['items', i, 'value']),


does it matter that this has to be a string, given that the "current" value can be a string, int, double, or bool?

You're right. This can be any given type. Fixed.

Hixie · 2023-12-27T04:48:37Z

packages/rfw/test/material_widgets_test.dart

@@ -135,15 +157,28 @@ void main() {
    await expectLater(
      find.byType(RemoteWidget),
      matchesGoldenFile('goldens/material_test.scaffold.png'),
-      skip: !runGoldens,
+      // skip: !runGoldens,


don't forget to uncomment this out

Oops. Uncommented.

packages/rfw/test/material_widgets_test.dart

Hixie · 2023-12-27T04:48:53Z

packages/rfw/test/material_widgets_test.dart

+    await expectLater(
+      find.byType(MaterialApp),
+      matchesGoldenFile('goldens/material_test.dropdown.png'),
+      // skip: !runGoldens,


packages/rfw/test/material_widgets_test.dart

Hixie · 2023-12-27T04:52:14Z

Added some comments, but overall looks pretty good!

Hixie · 2024-01-17T21:02:51Z

packages/rfw/CHANGELOG.md

@@ -2,6 +2,7 @@

 * Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
 * Fixes lint warnings.
+* Add one core widget and one material widget.


you should say which ones :-)
remember the old trick when writing documentation: what question is someone trying to answer when reading your prose? what questions will they have after reading your prose?

packages/rfw/lib/src/flutter/core_widgets.dart

Hixie · 2024-01-17T21:04:40Z

packages/rfw/lib/src/flutter/core_widgets.dart

+  'ClipRRect': (BuildContext context, DataSource source) {
+    return ClipRRect(
+      borderRadius: ArgumentDecoders.borderRadius(source, ['borderRadius']) ?? BorderRadius.zero,
+      // clipper,


looks like the convention for skipped parameters in this file is to give the type as well (see line 441)

packages/rfw/lib/src/flutter/material_widgets.dart

Hixie · 2024-01-17T22:13:28Z

Done! I updated the golden files as well. It seems disabled now though.

The goldens are Linux-only.

peixinli · 2024-01-23T23:48:19Z

Friendly ping:)

Hixie · 2024-01-24T00:53:17Z

@peixinli Sorry for not being clear. I was waiting for the test failures to be resolved before doing a final review pass. (No point giving an LGTM before the code is final, and the code is unlikely to be final if it's still failing tests. :-) )

peixinli · 2024-01-24T08:09:59Z

@peixinli Sorry for not being clear. I was waiting for the test failures to be resolved before doing a final review pass. (No point giving an LGTM before the code is final, and the code is unlikely to be final if it's still failing tests. :-) )

Done! Sorry I'm still learning the process.

Hixie

flutter/packages@8fbdf65...21b5abb 2024-01-24 pageli328@gmail.com Add rfw widgets (flutter/packages#5661) 2024-01-24 50643541+Mairramer@users.noreply.github.com [camera] add heif support image iOS (flutter/packages#4586) 2024-01-24 jsharp83@gmail.com [webview_flutter_android] Add javascript panel interface for android (flutter/packages#5796) 2024-01-24 43054281+camsim99@users.noreply.github.com [camerax] Wrap Android classes/methods required for implementing setting focus & exposure points and offset (flutter/packages#5659) 2024-01-24 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.2.0 to 4.3.0 (flutter/packages#5967) 2024-01-24 engine-flutter-autoroll@skia.org Roll Flutter from 5b673c2 to 19b06f4 (21 revisions) (flutter/packages#5968) 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-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Add two rfw widgets(ClipRRect and DropdownButton) and `flutter format` the widget files. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.) - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes]. - [x] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style]. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates [following repository CHANGELOG style]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests

peixinli added 2 commits December 12, 2023 22:05

add rfw widgets

2599faa

add changelog

2fee2ad

peixinli requested a review from Hixie as a code owner December 13, 2023 06:15

github-actions bot added the p: rfw Remote Flutter Widgets label Dec 13, 2023

Hixie reviewed Dec 16, 2023

View reviewed changes

peixinli added 4 commits December 21, 2023 16:05

revert format change

ba01940

add tests

77b49b9

add golden file

61bd40d

uncomment runGoldens line

9518814

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/lib/src/flutter/core_widgets.dart Outdated Show resolved Hide resolved

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/lib/src/flutter/material_widgets.dart Outdated Show resolved Hide resolved

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/lib/src/flutter/material_widgets.dart Outdated Show resolved Hide resolved

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/lib/src/flutter/material_widgets.dart Outdated Show resolved Hide resolved

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/test/material_widgets_test.dart Outdated Show resolved Hide resolved

Hixie reviewed Dec 27, 2023

View reviewed changes

packages/rfw/test/material_widgets_test.dart Show resolved Hide resolved

peixinli added 6 commits December 27, 2023 13:56

Fix comments and add more test

de5232c

Fix comments

e3248a6

test value type

06b3677

Merge branch 'main' into rfw

d5bcb37

add rfw widgets

5f031e1

add changelog

ad6f552

peixinli and others added 2 commits January 5, 2024 16:59

test value type

2ec30a2

Merge branch 'flutter:main' into rfw

a345557

Hixie reviewed Jan 17, 2024

View reviewed changes

packages/rfw/lib/src/flutter/core_widgets.dart Show resolved Hide resolved

Hixie reviewed Jan 17, 2024

View reviewed changes

packages/rfw/lib/src/flutter/material_widgets.dart Outdated Show resolved Hide resolved

Hixie reviewed Jan 17, 2024

View reviewed changes

packages/rfw/lib/src/flutter/material_widgets.dart Outdated Show resolved Hide resolved

peixinli and others added 4 commits January 17, 2024 13:32

fix comments

7ac2967

Merge branch 'flutter:main' into rfw

a23eeb1

Merge branch 'rfw' of https://github.com/peixinli/packages into rfw

751aedc

fix comments

a92cfcd

tugorez mentioned this pull request Jan 19, 2024

[RFW] Include more core, material and cupertino widgets in the rfw libraries. flutter/flutter#141668

Closed

peixinli and others added 7 commits January 23, 2024 21:26

merge main

372e27f

add line

32b8312

fix golden

43352e7

fix web

53a8a5b

fix

1f1db46

fix

16d1253

fix

e097047

Hixie approved these changes Jan 24, 2024

View reviewed changes

peixinli merged commit 21b5abb into flutter:main Jan 24, 2024
80 checks passed

engine-flutter-autoroll mentioned this pull request Jan 25, 2024

Roll Packages from 8fbdf65d0235 to 21b5abba7958 (6 revisions) flutter/flutter#142224

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 25, 2024

21b5abb Add rfw widgets (flutter/packages#5661)

54b8d82

reidbaker mentioned this pull request Feb 19, 2024

[in_app_purchase_android] Add UserChoiceBilling mode. #6162

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rfw widgets #5661

Add rfw widgets #5661

peixinli commented Dec 13, 2023

flutter-dashboard bot commented Dec 13, 2023

Hixie Dec 16, 2023

peixinli Dec 26, 2023

Hixie commented Dec 16, 2023

peixinli commented Dec 26, 2023 •

edited

Loading

Hixie Dec 27, 2023

peixinli Dec 27, 2023

Hixie Dec 27, 2023

peixinli Dec 27, 2023

Hixie Dec 27, 2023

peixinli Dec 27, 2023

Hixie Dec 27, 2023

peixinli Dec 27, 2023

Hixie Dec 27, 2023

Hixie commented Dec 27, 2023

Hixie Jan 17, 2024

Hixie Jan 17, 2024

Hixie commented Jan 17, 2024

peixinli commented Jan 23, 2024

Hixie commented Jan 24, 2024

peixinli commented Jan 24, 2024

Hixie left a comment


		// In these widgets we make an effort to expose every single argument available.
		Map<String, LocalWidgetBuilder> get _coreWidgetsDefinitions => <String, LocalWidgetBuilder>{

		/// * [DropdownButton] takes a list of items object with the key of params
		/// names and value of the param values to represent [DropdownMenuItem].

Add rfw widgets #5661

Add rfw widgets #5661

Conversation

peixinli commented Dec 13, 2023

Pre-launch Checklist

flutter-dashboard bot commented Dec 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hixie commented Dec 16, 2023

peixinli commented Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hixie commented Dec 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hixie commented Jan 17, 2024

peixinli commented Jan 23, 2024

Hixie commented Jan 24, 2024

peixinli commented Jan 24, 2024

Hixie left a comment

Choose a reason for hiding this comment

peixinli commented Dec 26, 2023 •

edited

Loading