realm / realm-studio

Realm Studio
https://realm.io/products/realm-studio/
Apache License 2.0
300 stars 40 forks source link

Wrong ErrorDialog being used when modifying the schema #713

Open cmelchior opened 6 years ago

cmelchior commented 6 years ago

If an error pops up that fills the entire screen it is not possible to remove it again. I have to close the current modal window completely.

image

I suspect the wrong widget is being used to display the error

bmunkholm commented 6 years ago

@cmelchior Didn't you fix this?

cmelchior commented 6 years ago

No, I added scrollbars. Also I'm not actually sure if this UI widget should be dismissed. We have another error dialog that can. Possible it is just the wrong UI widget being used in this case.

Perhaps @kraenhansen knows?

kraenhansen commented 6 years ago

Well this is a fatal error that occurred when opening the realm with an invalid schema. I don't know how to recover from that gracefully. It should probably fail earlier, when we know what the intent of the user is (did you add a property? A class? etc.).

There is a possibility to add a retry action to this overlay that would display a button under the message, but I doubt that we could implement a meaningful way of retrying this?

cmelchior commented 6 years ago

Actually, in the above case, that error came after trying to add a new class to a Synced Realm file if I remember correctly. Which is far from fatal, so my guess is that this should instead have been displayed using the ErrorMessageDialog component

I think it is fine to have a non-removable error screen if there is indeed use cases for it. I'll try to see if I can reproduce the sequence.

kraenhansen commented 6 years ago

The error overlay is a general overlay used when an error occurred while opening the Realm (with the modified schema). It's hard to reliably know what actually caused the issue.

To recover it provably needs to be re-opened without a schema, but as it's hard to determine what actually caused the error it's not clear if re-opening it will fix it.

In my opinion it's the API for schema manipulation that is not optimized for this use-case. Instead I would love a method on the Realm instance to add a new class that could fail more isolated if the new class cannot be added.

kraenhansen commented 3 years ago

I believe this will go away once https://github.com/realm/realm-studio/tree/kh/realm-browser-with-context gets merged.