Closed MarcWeber closed 8 months ago
Hi, thanks for the work! Could you go into a bit more detail about the changes here? Is it just conversion to NNBD, what else was done?
The first commit just tries to make all work. The following two try to update to latest libraries and allow null safety. -------- Ursprüngliche Nachricht --------Von: Shawn @.>Datum: Fr., 25. Nov. 2022, 21:55An: gskinnerTeam/flutter_vignettes @.>Cc: Marc Weber @.>, Author @.>Betreff: Re: [gskinnerTeam/flutter_vignettes] This tries to update the vignettes (PR #22) Hi, thanks for the work! Could you go into a bit more detail about the changes here? Is it just conversion to NNBD, what else was done?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>
Finally had some time to do a review on this. I think I can take it from here, but in general the issues are
late
it looks like many defaults can be set initially, which avoids the need for late
much of the time. ie, instead of late int foo
can just do int foo = 0
, or late List foo;
do List foo = data.items;
. This is just a little safer, as using late
opens the door to runtime errors when assignment is deferred to another line.int? foo
we can often do int foo = 0
, using too many nullable field leads to ...!
operator, mainly due to things being declared as nullable, rather than just get a default value. This also opens the door to runtime errors. Super helpful work though! Shouldn't take too long to do a bit of cleanup and land it.
My goal only was to have it run on latest Flutter to understand how it works. To take it from there I eventually some features/modules/ UI components even should be put into their own libraries. Cause you rarely use all. But still guessing. I totally agree that the work spent here might requires some more love in the future (cleanup).
I totally agree with your feedback.
@MarcWeber thank you for this PR. I'm able to run Flutter Vignettes 💙 on the latest version of Flutter 👍🏽
Closing this as we have another branch where this is happening now, thanks again for the 1st effort! https://github.com/gskinnerTeam/flutter_vignettes/pull/25
I am new to Flutter so maybe there are better ways to do it.
Also some demos only seem to have animations on the apps not Web targets (?).
Pick what you need as you see fit.
Also I noticed that the vignettes eventually could be improved by providing better apis such as the tabs using indexed lists .. Shouldn't there be an option to pass the contents via children: or alike ? ..
But its a repository to show off designs and that's what it does perfectly