Repository navigation
Breaking changes that would improve the overall API #24722
Description
Activity
- added this to the milestone
on Nov 26, 2018 May I propose this for the Framework too? #19143
I currently have some pretty ugly code to allow me to sequence an animation of changing tab and then scrolling to a certain position. This is because while
ScrollController.animateToreturns a Future so I can sequence something to happen after scrolling is done,TabController.animateTois void.Workaround (in the referenced issue above) is complicated and slightly absurd :-)
We should just be able to fix #19143, turning something from returning
voidto returning aFuturewhen it's something that we don't expect people to implement shouldn't be a breaking change.This bug is specifically about changes we can never really make because they're breaking changes.
Adding a type parameter to invokeMethod would be nice too: #17333Done, and non-breaking
May I complain here that I dislike how
Stackexpects its children ordered back-to-front in z-order? For example:Stack(children: [ child1, child2, child3, ])I find it rather unintuitive that
child1appears on top in the code but is actually at the bottom of the stack.Reacted by Casey RogersReacted by Todd Volkert, Shawn, Leonardo da Silva, Vinoth, Mateus Felipe C. C. Pinto, Nikodem Bernat, Jaroslaw Butajlo, Bartek Pacia, El and _juliotatiJust passing there after listening to your awesome podcast on itsallwidgets.com
ChangeNotifier and Listenable and company are a bit mixed up. We could refactor the code here so that we more clearly have a Listenable and a ListenableController, and a ValueListenable and a ValueListenableController, and so on.
What does it mean? Moving toward an API closer to
StreamvsStreamControllerforListenable?Container's width and height properties should size the child, and imply an alignment.
I'm not sure I agree with that one.
While the current behavior is definitely surprising for newcomers, that's a good thing IMO.From my experience mentoring in Flutter, that specific API is usually the first step that led peoples understanding that Flutter does layout differently.
This helped my mentees a lot in understanding that, in Flutter, UI elements means nothing without the rest of the tree – and that everything is declarative to the core.- added a commit that references this issue
on May 21, 2019 AnimatedWidget,ImplicitlyAnimatedWidgetand their subclasses should be named consistently. For the most part, subclasses of AnimatedWidget are namedFooTransitionand subclasses of ofImplicitlyAnimatedWidgetare namedAnimatedFoo. There are annoying/unnecessary exceptions, though, which should be renamed for consistency:AnimatedModalBarrier(subclass ofAnimatedWidget) should be renamed toModalBarrierTransitionAnimatedBuilder(subclass ofAnimatedWidget) should be renamed to something likeTransitionBuilder*)AnimatedWidgetitself should probably more accurately be namedTransitionWidget- Maybe:
ImplicitlyAnimatedWidgetshould just beAnimatedWidget - Name the
FoovsFooTransitionvsAnimatedFoowidgets more consistently (e.g.OpacityvsFadeTransitionvsAnimatedOpacity).
*) This would also fix another item on the list above: "Rename AnimatedBuilder to make it clear it takes any Listenable (even AnimationBuilder would be more consistent...)."
Reacted by Kate Lovett, Marcel Garus and Nate WilsonPlease feel free to edit the list above so that we just have one comment with all the proposed changes.
@Hixie I added a bullet to the list in the top post and linked to the detailed description in the comments.
@goderbauer is the animations group planning on renaming some of these like you mentioned (such as
AnimatedModalBarrier)?One other to add to the list of naming mismatches is
AnimatedSize.I added AnimatedSize to the list above.
I did not have plans to rename these. It feels like the change would be rather disruptive to current users of these without any benefit for them?
When we look to do a Flutter 2.0 release, we should consider doing the things listed in this issue as part of the 1.x->2.x break.
Reacted by Kate Lovett, Pedro Massango, thecalamiity, Mateus Felipe C. C. Pinto and Casey Rogers21 remaining items
It seems to me that some naming-related items on this list could be done by renaming the offending identifiers, and introducing a temporary (for ~1 year/4 major releases) type-alias with the old name.
For example:
- Adapter -> Adaptor
- BlendMode.srcATop -> BlendMode.srcAtop
- BlendMode.dstATop -> BlendMode.dstAtop
Some things I felt like adding here:
- Convert
Builderclasses into constructors and make thebuilderargument positional
BuilderandStatefulBuildercan be useful, but there's no benefit to extending those classes; instead they could beWidget.builderandWidget.statefulBuilder.// before child: Builder(builder: (context) { return FooWidget(); }) // after child: .builder((context) { return FooWidget(); })
Freeing up the namespace would be beneficial, e.g. a developer (or this framework) could use
Builderas a class or typedef without a name overlap.
- Combine
Animatable<T>andParametricCurve<T>
Animatablehas atransformmethod which creates a value based on adouble tparameter (a fraction in the range 0.0 to 1.0);ParametricCurve.transform()does pretty much exactly the same thing.
But a few differences in the implementation make it so we can't just have one of those classes as a subtype of the other without breakages.
- Get rid of
PreferredSizeWidget
Any logic that needs the dimensions of one or more children should handle it during
RenderObject.performLayout.
- Rework
AutomaticKeepAliveClientMixin
Instructing developers to call
super.build(context)and ignore the return value is unintuitive API design IMO. I'd prefer one of the following:- Introduce a public method that returns
voidinstead of overridingbuild - Make it a
mixin on StatefulWidget: you'd override thewantKeepAlivegetter on the widget class, and the mixin would changecreateElementso that no changes to the state'sbuildmethod are needed.
-
Separate packages for framework layersit came true! 🙂
- Convert
could you please add some gesture information to the hitTest method for custompainter??? for example when using interactiveViewer and painting a list of items, say like a map with points of interest, i need to detect taps on those items.
hitTest gets triggered when panning on the map, causing accidental clicks. if hitTest has some info about which gesture happened i could make it work. Current workaround is to wrap CustomPaint in a GestureDetector and use onTapDown.
I think you might be using
hitTestwrongly. It's purpose is not to tell you that your widget was tapped, so you can call a callback, or trigger some logic. It's purpose is for you to inform the framework that there's something in those coordinates, so the gesture detection algorithm can decide which widget is being tapped (as you might be tapping on a transparent area in your widget, so the widget that's behind can get that gesture instead), and will get the gesture callback activated.Reacted by Nate WilsonDevelopers often use
ValueChanged<T>in contexts where the function isn't being called on changes and has nothing to do with changing values-they just wanted a type def forvoid Function(T)andValueChangedwas handy.As such,
ValueChanged<T>should've just been calledValueCallback<T>to make it more obviously usable outside of strictly on change callbacks.Reacted by Bartek PaciaDevelopers often use
ValueChanged<T>in contexts where the function isn't being called on changes and has nothing to do with changing values-they just wanted a type def forvoid Function(T)andValueChangedwas handy.As such,
ValueChanged<T>should've just been calledValueCallback<T>to make it more obviously usable outside of strictly on change callbacks.This can be no breaking. If found valuable, we can introduce one without removing the other. They would be virtually equal, but have different semantics.
I agree this would be a nice addition, in the mean time remember that the framework also provides
ValueSetter<T>.Another one: #187909
WidgetControllerfrompackages/flutter_test/lib/src/controller.dartshould not be using a static variable_nextPointeras a counter. It is making the tests dependent on each other in some specific scenarios.Reacted by Justin McCandless
This bug lists changes we'll probably never make, but should consider if we ever for some reason decided to start over in a new universe.
Foundation
ChangeNotifierandListenableand company are a bit mixed up. We could refactor the code here so that we more clearly have aListenableand aListenableController, and aValueListenableand aValueListenableController, and so on.Services
StandardMessageCodecshould be a static class or should be stateful.dev.flutter/channel-bufferscontrol messages should use a more efficient binary format rather than an inefficient bespoke\r-delimited text-based format.flutter/system,flutter/navigation, etc, control messages should use a more efficient binary format rather than JSON.Painting
addListener/removeListeneronImageStreamhave a broken API that should be redesigned.NotchedShapeshould be a shape, or should have another name.TextStylehas many decoration properties, it would be nice if they were all in one object, maybe separate objects for underline and overline and strike-through.Gestures
PointerMoveEventtoPointerUpdateEvent, because it's also triggered at button changes without moves.Animation
Tweens should be immutable.vsyncin AnimationController totickerProvider(Roll engine c41bbc63d955cc7056a162957ac4eeeca6647dc2..e77e4f0e051cb53754814ffe33df291a551bdba3 (5 commits) #37255)AnimationController, have one for each type of controller (bounded, unbounded, simulation, repeating, etc).Animatable.animatein favour ofAnimation.drive.CurveTweenconstructor's argument should be positional. (Make CurveTween curve a regular parameter #21433)Rendering
Adaptor->Adapterchildrenargument instead of a positional oneCustomPainter.hitTestshould take aSizeargument (CustomPainter.hitTest missing size argument #28206)Layer.pushLayershould be renamed towithLayer, since it is not matched by apopLayer. The same applies to its relatives.BlendMode.srcATop->BlendMode.srcAtopBlendMode.dstATop->BlendMode.dstAtopWidgets
WidgetBuilderto make it clear it's a typedef.TransitionBuilderto make it clear it's a typedef with a child.AnimatedBuilderto make it clear it takes anyListenable(evenAnimationBuilderwould be more consistent...), see also: Breaking changes that would improve the overall API #24722 (comment).Listenerto bePointerListenerAnimatedWidgetandImplicitlyAnimatedWidgetshould be named consistently (see Breaking changes that would improve the overall API #24722 (comment)).AnimatedSize).FooBuildertypedefs into one generic typedef, or otherwise cleaning up the inconsistencies there.inheritFromWidgetOfExactTypeshould be generic rather than taking aType(so it can guarantee its return type is of the same type as the argument).Container's width and height properties should size the child, and imply an alignment.showDialogandshowGeneralDialog.AnimatedListshould beAnimatedListView.SliverMultiBoxAdaptorWidgetis not an adaptor; consider naming likeSliverMultiChildRenderObjectWidget.RichTexttoRawText.maxLinesfrom int to double (TextField maxLines should be a double, not an int. #35064).ColorFiltered->ColorFilterFocusNodeandFocusScopeNodeshould use 'focusable' instead of 'canRequestFocus' as the attribute for controlling focusability.Element.activateshould rename toElement.reactivate, because only the reactivate will callback.OnInvokeCallbackshould be called justInvokeCallbackper the style guide.reportsRouteUpdateToEngineshould bereportRouteUpdateToEngine(imperative not descriptive).Material
InputDecoration.filledshould beisFilledInputDecorationshould only have one prefix field and one suffix field, rather than three each (widget, icon, text). And it probably should have anerror, alabel, and ahintinstead of justerrorText,labelText, andhintText.DividerandVerticalDivider'sindentandendIndentproperties to simply bemarginColors.grey->Colors.gray