Conversation
Hohrenda
left a comment
Hohrenda
left a comment
There was a problem hiding this comment.
Well done! I've added a couple of naming suggestions - please take a look
|
Added Provider and some features for clean arch |
Hohrenda
left a comment
Hohrenda
left a comment
There was a problem hiding this comment.
Great work adding the Provider package. I added a couple of suggestions - please take a look
|
Add feature: database |
Hohrenda
left a comment
Hohrenda
left a comment
There was a problem hiding this comment.
Looks good! There are some minor suggestions though.
| export 'constants.dart'; | ||
| export 'custom_button.dart'; | ||
| export 'custom_drop_button.dart'; | ||
| export 'custom_dropdown_items.dart'; | ||
| export 'custom_text_field.dart'; | ||
| export 'theme.dart'; No newline at end of file |
There was a problem hiding this comment.
We usually don't do this because there might be cases where not all imports from here are needed. I would prefer importing each widget on demand.
| contentPadding: | ||
| const EdgeInsets.symmetric(horizontal: 20, vertical: 5), | ||
| labelText: labelText, |
There was a problem hiding this comment.
Some int values are used multiple times. What do you think about creating a class called "SizeDimens" with frequently used values?
| Future.delayed(Duration.zero, () { | ||
| Navigator.pushNamedAndRemoveUntil( | ||
| context, '/main', (Route<dynamic> route) => false); | ||
| }); |
There was a problem hiding this comment.
Can you please elaborate on why the Future.delayed is used here?
| Future.delayed(Duration.zero, () { | ||
| Navigator.pushNamedAndRemoveUntil( | ||
| context, '/', (Route<dynamic> route) => false); | ||
| }); |
| export 'login.dart'; | ||
| export 'signup.dart'; | ||
| export 'verify_email.dart'; No newline at end of file |
There was a problem hiding this comment.
We usually don't do this because there might be cases where not all imports from here are needed. I prefer to import each widget on demand.
| case 'email-already-in-use': | ||
| return AuthErrorCode.emailAlreadyInUse; | ||
| case 'user-not-found': | ||
| case 'wrong-password': |
There was a problem hiding this comment.
These cases can also be moved to the enum. Flutter supports enums with value:
| onTap: () { | ||
| // Navigator.pushNamed(context, '/navigate'); | ||
| }, |
2 participants
Add some features to a clean arch:
@Hohrenda