Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create custom error screen #297

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: create custom error screen #297

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac1ed44
create Error screen
Zfinix May 19, 2022
a51e290
pass error to erro widget
Zfinix May 19, 2022
91bd60b
pass error to widget
Zfinix May 19, 2022
331bfea
fix minor lint bug
Zfinix May 23, 2022
ecf42f4
create CosmosErrorDetails entity
Zfinix May 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions starport_template/lib/pages/create_account_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class _CreateAccountPageState extends State<CreateAccountPage> {

bool get isAccountImportingError => StarportApp.accountsStore.isAccountImportingError;

String get errorDetails => StarportApp.accountsStore.errorDetails;

@override
void initState() {
super.initState();
Expand Down Expand Up @@ -122,8 +124,8 @@ class _CreateAccountPageState extends State<CreateAccountPage> {
}

Widget _errorUI() {
return const Center(
child: Text('Error!'),
return ErrorWidget.withDetails(
error: FlutterError(errorDetails),
);
}

Expand Down Expand Up @@ -193,6 +195,7 @@ class _CreateAccountPageState extends State<CreateAccountPage> {
..add(DiagnosticsProperty<bool>('isLoading', isLoading))
..add(DiagnosticsProperty<bool>('isAuthenticating', isAuthenticating))
..add(DiagnosticsProperty<bool>('isAccountImportingError', isAccountImportingError))
..add(DiagnosticsProperty<bool>('isError', isError));
..add(DiagnosticsProperty<bool>('isError', isError))
..add(DiagnosticsProperty<String>('errorDetails', errorDetails));
}
}
47 changes: 46 additions & 1 deletion starport_template/lib/starport_app.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'package:alan/alan.dart';
import 'package:cosmos_auth/auth/cosmos_auth.dart';
import 'package:cosmos_ui_components/cosmos_theme.dart';
import 'package:cosmos_ui_components/cosmos_ui_components.dart';
import 'package:cosmos_utils/cosmos_utils.dart';
import 'package:flutter/material.dart';
import 'package:flutter_mobx/flutter_mobx.dart';
Expand Down Expand Up @@ -34,6 +34,7 @@ class StarportApp extends StatelessWidget {
brightness: themeStore.isDarkTheme ? Brightness.dark : Brightness.light,
child: Builder(
builder: (context) {
setErrorBuilder(context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really sure why we're setting a variable as part of the build method, and why this variable is static? Looks like flutter team at google was a bit drunk when implemented this O.o. :D Wouldn't it make more sense to have specialized widget that is responsible of building the Error UI ? I think ErrorWidget is more meant for general build errors and altering it's builder field you're updating the way all build errors are being displayed across the whole app.

Copy link
Contributor Author

@Zfinix Zfinix May 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.flutter.dev/testing/errors, yes they were drunk, I figured it would be great if we had our own custom error screen app-wide, gets rid of the red screen of death and allows us to still copy error messages with one click of a button

return MaterialApp(
title: 'Starport template',
theme: CosmosTheme.of(context).buildFlutterTheme(),
Expand All @@ -44,4 +45,48 @@ class StarportApp extends StatelessWidget {
),
);
}

void setErrorBuilder(BuildContext context) {
ErrorWidget.builder = (errorDetails) {
return Material(
color: Colors.white,
child: Center(
child: Padding(
padding: const EdgeInsets.all(24),
child: ListView(
children: [
const Text(
'Error!',
style: TextStyle(
color: Colors.white,
fontWeight: FontWeight.bold,
),
),
Text(
errorDetails.toString(),
style: const TextStyle(
color: Colors.white,
fontWeight: FontWeight.w400,
),
),
Row(
children: [
CosmosElevatedButton(
text: 'Take me back',
onTap: () => Navigator.pop(context),
textColor: Colors.black,
),
CosmosTextButton(
text: 'Back',
onTap: () => Navigator.of(context).pop(),
),
],
),
],
),
),
),
);
};
}
}
8 changes: 8 additions & 0 deletions starport_template/lib/stores/accounts_store.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class AccountsStore {
final Observable<bool> _isBalancesLoadingError = Observable(false);
final Observable<bool> _isRenamingAccount = Observable(false);
final Observable<bool> _isSendingMoney = Observable(false);
final Observable<String> _errorDetails = Observable('');

final ObservableList<Balance> balancesList = ObservableList();
final Observable<CredentialsStorageFailure?> loadAccountsFailure = Observable(null);
Expand Down Expand Up @@ -78,6 +79,10 @@ class AccountsStore {

set isMnemonicCreatingError(bool val) => Action(() => _isMnemonicCreatingError.value = val)();

String get errorDetails => _errorDetails.value;

set errorDetails(String val) => Action(() => _errorDetails.value = val)();

bool get isMnemonicCreating => _isMnemonicCreating.value;

set isMnemonicCreating(bool val) => Action(() => _isMnemonicCreating.value = val)();
Expand Down Expand Up @@ -202,6 +207,7 @@ class AccountsStore {
return result.fold(
(fail) {
logError(fail);
errorDetails = fail.toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're loosing a lot of info here by calling toString(), couldn't we simply make errorDetails a, lets say ErrorDetails type that consists of the dynamic error and optional StackTrace? stackTrace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update this to use ErrorDetails

Copy link
Contributor Author

@Zfinix Zfinix May 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created CosmosErrorDetails

isAccountImportingError = true;
return null;
},
Expand Down Expand Up @@ -234,6 +240,7 @@ class AccountsStore {
await getBalances(selectedAccount.publicAddress);
} catch (ex, stack) {
logError(ex, stack);
errorDetails = '$ex\n\nStackTrace:\n\n$stack';
isSendMoneyError = true;
}
isSendMoneyLoading = false;
Expand Down Expand Up @@ -271,6 +278,7 @@ class AccountsStore {
mnemonic = await generateMnemonic();
} catch (ex, stack) {
logError(ex, stack);
errorDetails = '$ex\n\nStackTrace:\n\n$stack';
isMnemonicCreatingError = true;
}
isMnemonicCreating = false;
Expand Down