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

Improve connection status #42

Merged
merged 11 commits into from
Jan 4, 2024
Merged

Improve connection status #42

merged 11 commits into from
Jan 4, 2024

Conversation

rkistner
Copy link
Contributor

@rkistner rkistner commented Jan 2, 2024

Changes to SyncStatus:

  • Fix connected not updating when calling powersync.disconnect().
  • Add connecting, downloading and uploading flags.
  • Add downloadError, uploadError and a combined anyError field.

The demo now uses these to provide more fine-grained connectivity status.

image

Also includes a minor fix for error parsing that would only return the first character of the error message in some cases.

@@ -1,5 +1,6 @@
import 'dart:async';

import 'package:flutter/foundation.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on all the repeated changes should we not just create a common component for that status bar and then have all the demos use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically the status bar is slightly different between demos (one contains search), but the status icon is the same and could be re-used.

I'm a little conflicted on this one: One the one side a common package would allow us to re-use code between the demos, but on the other side it makes it more difficult for others to copy and modify a demo.

What I think would actually be best is to reduce the scope of some of the demos (specifically supabase-anonymous-auth and supabase-edge-auth) to not contain the entire todolist app, but rather just a minimal amount of code to show how to use custom auth functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that makes sense. I left the status bar in the auth demos because of the status icon mainly but don't think it's really necessary

kobiebotha
kobiebotha previously approved these changes Jan 3, 2024
@rkistner rkistner requested a review from DominicGBauer January 3, 2024 12:51
@rkistner rkistner merged commit aff84cd into master Jan 4, 2024
4 checks passed
@rkistner rkistner deleted the improve-status branch January 4, 2024 07:20
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