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.

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

wants to merge 5 commits into from

Conversation

Zfinix
Copy link
Contributor

@Zfinix Zfinix commented May 19, 2022

I have created a default error screen using flutter's inbuilt error widget and i have also passed the error from account creation to it.

Widget _errorUI() {
    return ErrorWidget.withDetails(
      error: FlutterError(errorDetails),
    );
  }

Global Error UI:

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(),
                    ),
                  ],
                ),
              ],
            ),
          ),
        ),
      );
    };
  }

@Zfinix Zfinix changed the title Feat: create Error screen feat: create custom error screen May 19, 2022
@Zfinix Zfinix requested review from andrzejchm and wal33d006 and removed request for andrzejchm May 23, 2022 06:42
Copy link
Contributor

@wal33d006 wal33d006 left a comment

Choose a reason for hiding this comment

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

Looks good to me, can we make it support CosmosTheme? Currently the background color and text colors are all fixed.

@@ -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

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

@@ -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

Choose a reason for hiding this comment

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

created CosmosErrorDetails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants