[listen] Creates listen package#24
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This introduces the key parts of a paginated data table, not including the built-in pagination features. * Provide more data for the data table demo, so there's data to page. * Introduce a ChangeNotifier class which abstracts out addListener/removeListener/notifyListeners. We might be able to use this to simplify existing classes as well, though this patch doesn't do that. * Introduce DataTableSource, a delegate for getting data for data tables. This will also be used by ScrollingDataTable in due course. * Introduce PaginatedDataTable, a widget that wraps DataTable and only shows N rows at a time, fed by a DataTableSource.
This patch improves some subtle behaviors about the change notifier.
…889) Having this base class lets classes like CustomPainter and DataTableSource be more agnostic as to what's generating the repaints.
Sometimes you have several listenables, but you want to hand them to an API (e.g. CustomPainter) that only expects one.
This lets you use `Listenable.merge` without having to sanitize your incoming list of change notifiers, in case your semantics are that they are optional.
* MultiTapGestureRecognizer previously would assert if there was no competition. * GestureArenaTeam would always select the first recongizer as the winner even if a later recognizer actually accepted the pointer sequence. * debugPrintStack would fail a type check if maxFrames was non-null. * FractionalOffset.lerp would throw a null-pointer exception if its second argument was null. Also, add a number of tests for previously untested lines of code.
It took me a while to figure out what was going on (I was removing a listener after disposal). These asserts helped.
This patch aligns the iteration patterns used by animations and ChangeNotifier. They now both respect re-entrant removal of listeners and coalesce duplication registrations. (Also, ChangeNotifier notification is no longer N^2). Fixes #7533
This patch aligns the iteration patterns used by animations and ChangeNotifier. They now both respect re-entrant removal of listeners and coalesce duplication registrations. (Also, ChangeNotifier notification is no longer N^2). This patch introduces ObserverList to avoid the performance regression that the previous version of this patch caused. Fixes #7533
Fixes for: flutter/flutter#7570 flutter/flutter#7231 flutter/flutter#2841 and others
It's common to have a ChangeNotifier that wraps a single value. This class makes that easy by providing a generic implementation.
* Change foundation references to meta * Remove specified shows
After this patch, there are three major text input widgets: * EditableText. This widget is a low-level editing control that interacts with the IME and displays a blinking cursor. * TextField. This widget is a Material Design text field, with all the bells and whistles. It is highly configurable and can be reduced down to a fairly simple control by setting its `decoration` property to null. * TextFormField. This widget is a FormField that wraps a TextField. This patch also replaces the InputValue data model for these widgets with a Listenable TextEditingController, which is much more flexible. Fixes #7031
…e method. (#10871) * Change all instances of '$runtimeType#$hashCode' to use the describeIdentity method. The describeIdentity method generates a shorter description with a consistent length consisting of the runtime type and the a 5 hex character long truncated version of the hash code.
…idAndType method. (#10871)" (#10880) This reverts commit d6afe09.
…eIdentity (#10888) * Revert "Revert "Change all ocurrences of '$runtimeType#$hashCode' to use the idAndType method. (#10871)" (#10880)" This reverts commit 7ccc66f.
Add Diagnosticable base class and documentation
I got tired of drive-by spelling fixes, so I figured I'd just take care of them all at once. This only corrects errors in the dartdocs, not regular comments, and I skipped any sample code in the dartdocs. It doesn't touch any identifiers in the dartdocs either. No code changes, just comments.
* Fix the confusing-zero case with NestedScrollView. * Update mock_canvas.dart * Update tabs_demo.dart * more tweaks
I found that some ValueListeners want to know when they should start doing work (e.g. if the value comes from polling a network resource).
2118fe9 to
a2354b4
Compare
stuartmorgan-g
left a comment
There was a problem hiding this comment.
I only looked at the changes to fit into the repo structure; someone more familiar with the intended usage and the migration plan for clients should definitely review the package API itself.
| description: A lightweight synchronous listener registry for pure Dart applications. | ||
| repository: https://github.com/flutter/core-packages/tree/main/packages/listen | ||
| issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+listen%22 | ||
| version: 0.1.0 |
There was a problem hiding this comment.
It looks like someone published a placeholder package as 1.0.0-beta, which is going to make publishing a 0.x release confusing.
|
|
||
| /// Demonstrates [ValueNotifier] usage for README. | ||
| // #docregion ValueNotifier | ||
| void valueNotifierExample() { |
There was a problem hiding this comment.
Is there a reason this is duplicated almost exactly here from main.dart, instead of the README snippet just pulling from main.dart?
There was a problem hiding this comment.
That will require adding the #docregion identifier on the main.dart, which I think may be confusing for people looking at the example. so I intentionally group all excerpt related code in one file even if I can probably grab the code from some other example. Let me know reducing duplication here is more preferred.
There was a problem hiding this comment.
Traditionally we haven't worried about having it in main.dart, and only use readme_excerpts.dart when there were things we wanted to show in the README that weren't in the example.
It's not a rule though, so if you are concerned about the comments in the example this is fine.
There was a problem hiding this comment.
I will keep this as-is
| } | ||
| } | ||
|
|
||
| /// Demonstrates [ChangeNotifier] usage for README. |
There was a problem hiding this comment.
Nit: can we remove this line? Or could we split this into two excerpts so that we can skip this line?
|
|
||
| # listen | ||
|
|
||
| A lightweight synchronous listener registry for pure Dart applications. |
There was a problem hiding this comment.
This line feels a little abstract and might be difficult to understand if you have no context. What do you think of this instead?
| A lightweight synchronous listener registry for pure Dart applications. | |
| A package to notify state changes to interested listeners. |
ecf0f23 to
da26a37
Compare
|
|
||
| group('readme excerpts', () { | ||
| test('valueNotifierExample runs without error', () { | ||
| expect(readme_excerpts.valueNotifierExample, returnsNormally); |
There was a problem hiding this comment.
Can we do Zone magic here to check what is printed? That'll ensure that when users run the samples, the output they see matches the comments.
loic-sharma
left a comment
There was a problem hiding this comment.
This looks good to me, but please also get an approval from Stuart :)
| @@ -0,0 +1,3 @@ | |||
| ## 1.0.0-beta.1 | |||
|
|
|||
| - Moves source code from `flutter/flutter` to `flutter/core-packages`. | |||
There was a problem hiding this comment.
This isn't just a move, it's an extraction as a stand-alone package. We should clarify what the full scope of the change is.
There was a problem hiding this comment.
I just realized that changing this would require a new CLA override. Feel free to just adjust in in the next PR to this package.
There was a problem hiding this comment.
updated since i will have to squash new commits into a single one and will require a CLA override anyway
| @@ -0,0 +1,3 @@ | |||
| ## 1.0.0-beta.1 | |||
There was a problem hiding this comment.
We never do releases like this, so I'm not entirely sure our CI will handle it correctly. It looks like presubmit is fine, so fingers crossed, but be sure to verify that publishing actually worked after it lands.
|
I've submitted a CLA override for the import, using the usual explanation of this being an import from another repo in the same org that already enforced the CLA on these contributions. |
da26a37 to
0be3949
Compare
|
@stuartmorgan-g , I may need some help to merge this with merge commit to preserve the history. |
|
@chunhtai Is it ready to land? If so I can adjust the settings and merge it. |
| /// | ||
| /// By default, errors are thrown as [StateError]. | ||
| static ErrorCallback onError = (String message, StackTrace? stackTrace) { | ||
| throw StateError(message); |
There was a problem hiding this comment.
Would it make sense to instead use Zone.current.handleUncaughtError and send back the original err/stack?
There was a problem hiding this comment.
I am not sure whether Zone.current.handleUncaughtError is the best way to handle this as it handles the error async, thus won't halt immediately. I do agree the original stack should be preserve somehow. I will handle this as a follow up after this pr
|
@stuartmorgan-g yes, this is ready to land |
code are striped from the flutter/flutter with history intact.
The change is in last commit
related to flutter/flutter#149466
Pre-Review Checklist
[vector_math]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2