Increase widget customization #22
Conversation
|
Hi @Macacoazul01 , very nice PR and I'm grateful for that. We can accept your PR if you split your commits and follow our https://github.com/ofload/native_updater/blob/master/CONTRIBUTING.md guidelines. It's a simple thing but helps us to keep the code and changes organised and we are able to reason and decide what every change needs or causes before releasing a new version of the plugin. From your changes, I can see you could split your commits into 3. 1- Update documentation and example files I will comment on individual lines for more clarity. |
| @@ -27,12 +27,11 @@ class _HomeState extends State<Home> { | |||
| /// response of HTTP request. | |||
| /// Let's say the statusCode 412 requires you to force update | |||
| Future.delayed(Duration.zero, () { | |||
| if (statusCode == HttpStatus.unauthorized;) { | |||
| if (statusCode == HttpStatus.unauthorized) { | |||
| ); | ||
| } else if (serverLatestVersion > localVersion) { | ||
| NativeUpdater.displayUpdateAlert( | ||
| context, | ||
| forceUpdate: false, | ||
| appStoreUrl: '<Your App Store URL>', | ||
| playStoreUrl: '<Your Play Store URL>', |
There was a problem hiding this comment.
because it isnt used on the code, just to be put on the debug log
| @@ -9,7 +9,8 @@ environment: | |||
| dependencies: | |||
| flutter: | |||
| sdk: flutter | |||
| cupertino_icons: ^0.1.3 | |||
| cupertino_icons: ^1.0.3 | |||
| dio: ^4.0.0 | |||
There was a problem hiding this comment.
the example files are not meant to run, so don't need dependencies. If you are able to run it and think this will make others life easier, would you mind adding instructions on our README on how to run them?
| @@ -3,10 +3,14 @@ import 'package:flutter/material.dart'; | |||
| class ErrorMaterialAlert extends StatelessWidget { | |||
| final String appName; | |||
| final String description; | |||
| final String? errorCloseButtonLabel; | |||
There was a problem hiding this comment.
great! could have its own commit with why in the commit message.
lib/src/native_updater.dart
Outdated
| /// In App Update Related | ||
| try { | ||
| AppUpdateInfo _updateInfo = await InAppUpdate.checkForUpdate(); | ||
|
|
||
| if (_updateInfo.updateAvailability == UpdateAvailability.updateAvailable) { | ||
| if (_updateInfo.updateAvailability == |
There was a problem hiding this comment.
that's not a good formatting, does not seems to follow the standard. A single line is preferred in this case.
| @@ -55,7 +58,7 @@ class UpdateMaterialAlert extends StatelessWidget { | |||
| crossAxisAlignment: CrossAxisAlignment.start, | |||
| children: <Widget>[ | |||
| Text( | |||
| 'New version available', | |||
| newVersionLabel, | |||
| @@ -12,9 +12,9 @@ dependencies: | |||
| flutter: | |||
| sdk: flutter | |||
| # package_info will help us to find current installed version of application. | |||
| package_info: ^2.0.0 | |||
| package_info: ^2.0.2 | |||
There was a problem hiding this comment.
changes in dependencies need to be justified and deserve and individual commit. Only if the dependencies update are needed to allow the changes, then they should be together but this does not seem to be the case.
I can't do this right now, it takes time to comment on everything and make 3 prs instead of one. You guys can use this pr as an suggestion to create or own new update. The point of this PR is to make the library useful on other languages instead of just english (item 2). The item 1 i've made because it was broken and the 3 just to push to the most recent ones |
No description provided.