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

List creation and divider issue #15

Closed
stemuk opened this issue Jul 24, 2018 · 14 comments
Closed

List creation and divider issue #15

stemuk opened this issue Jul 24, 2018 · 14 comments

Comments

@stemuk
Copy link

stemuk commented Jul 24, 2018

I took some time off last weekend to test the Zefyr editor and to record some footage of the issues that I encountered. During that process I found two rather minor issues, one concerning the creation of ordered and unordered lists, one concerning the divider functionality.

List creation issue: https://streamable.com/62rn4
Divider issue: https://streamable.com/ze8yj

The issue with the List creation only seems to occur on non-emulated android devices (the footage was recorded on a physical Android 7.1.1 device), since I couldn't reproduce it on my emulated Android device.
The Divider issue can be reproduced on emulated Android devices as well, and throws the error:

W/IInputConnectionWrapper( 3850): getTextBeforeCursor on inactive InputConnection

which would make me think that it is caused by a race condition.

Edit: I just looked at the footage again and it seems like zefyr has a problem with auto corrected text, both in the List creation issue and when you mark text as bold/italic.
Lists creation seems to work fine until you have an autocorrect suggestion and decide NOT to use it and press enter. In this case the autocorrect suggestion seems to be used anyway (and not the original text you typed) and the error occurs.

@pulyaevskiy
Copy link
Contributor

Thanks, this is very helpful! I will try to reproduce it on my end and look for a fix.

Just to confirm, you used zefyr 0.1.1 in these tests, right?

@stemuk
Copy link
Author

stemuk commented Jul 25, 2018

Yes, however I just tried out 0.1.2 and the issue can still be reproduced.

Edit: As I mentioned above, I think that the issues can be reproduced best on a physical Android device with G-Board, Google's auto suggestion keyboard. I think it comes preinstalled on pretty much any new device, and the issue seems to be caused by the autosuggestions.

@pulyaevskiy
Copy link
Contributor

Yeah, I didn't expect 0.1.2 to address this issue. Thanks for confirming anyway!

I don't have an Android device yet, but will try to dig into this issue a bit later this week. And I'm planning to get something running on Android soon as well.

@stemuk
Copy link
Author

stemuk commented Jul 26, 2018

Ok, if you need any more info about the issues or need better footage let me know!

I got two small questions while we're at it, a) is there a way to change the current NotusDocument via the contoller, like _controller.changeDocument(myNotusDocument), and b) would it make sense to save changes to the documents by listening to changes and then updating the entire database entry with db.update('document1': _contoller.document) or is there a better way?

Edit: just checked again with an Android Emulator and it seems the list issue can be reproduced just as easily with an Emulator by typing and hitting enter as soon as GBoard suggests an autocorrection entry (the suggeested word is shown in bold text, not in the regular font style). So an Android device shouldn't be necessary for reproducing these issues. 👍

@pulyaevskiy
Copy link
Contributor

a) There is no API which allows this. It could be tricky because users can subscribe to changes on original document so replacing it completely can lead to unexpected behavior. You could potentially replace controller itself but I haven't tested this thoroughly. Curious about your case for replacing entire document?

b) Not sure I fully understand this scenario. If you need to store a document in a database you simply get current state from the controller, as in your example. changes stream is useful if you are interested in individual updates to the document. It is possible to reconstruct entire document from this stream. If you have a more complete example it might help me to understand this problem better.

@stemuk
Copy link
Author

stemuk commented Jul 26, 2018

Thanks for answering my questions!
As for a) my current layout would include a drawer that holds several ListTiles (the documents) and ontap() should change Zefyr to show the selected document. This is why I was looking for a way to change (i.e replace) the current document shown by the editor, since my intention is to essentially re-render Zefyr and replace the previously shown document with the newly selected one.

As for b): I think having the _controller.document.changes Stream should work, I will just have to make a way work to save the Stream efficiently to the database. Saving the entire document to the database after every single change is not very efficient, so only updating the part that changed is probably a good idea.

@pulyaevskiy
Copy link
Contributor

I think you should be able to achieve a) by creating new controller on every switch + giving ZefyrEditor unique Key based on some identifier you have for current document (probably ID from database record). Need to be careful with subscriptions to [changes] stream and make sure old ones are canceled and new ones are established.

For b) you can leverage rxdart and debounce changes to emit an event once a second or similar, e.g.

final snapshots = document.changes.map((NotusChange change) {
  return change.before.compose(change.change);
});
final observable = new Observable(snapshots);
observable.debounce(const Duration(seconds: 1)).listen((Delta snapshot) {
  // triggered every second, [snapshot] contains latest document state.
db.update(docId, snapshot);
});

Note that this introduces 1 second delay so if users switches between two documents too fast after making a change then this last change may not reach your persistence handler.

@pulyaevskiy
Copy link
Contributor

I've just pushed a small update to master branch which should fix one edge case with handling composing updates, not sure if it fixes the above issues though.

If you'll have a chance to try it out it'd be great.

I've also created this issue which seems to be slightly related as well: flutter/flutter#20016

@stemuk
Copy link
Author

stemuk commented Jul 31, 2018

Sure, would you mind pushing 0.1.3 to pub.dartlang.org, this would probably simplify testing. If it isn't ready yet I could of course try the github version, but loading with pub get always seems so much easier 👍

On a side note I forgot to thank you for your previous answer, I tried rxdart out today but I had trouble finding a spot to run the example you gave. The scope of the variables like _controller doesn't allow me to execute the code in main() , should I execute the example in class _MyHomePageState extends State<MyHomePage> in void initState() instead?

@pulyaevskiy
Copy link
Contributor

0.1.3 is not quite ready yet but you can install it with pub by adding dependency override in your pubspec. E.g.:

# Somewhere below dependencies
dependency_overrides:
  zefyr:
    git:
      url: https://github.com/memspace/zefyr.git
      path: packages/zefyr

Note that there is a breaking change in ZefyrImageDelegate, though if you haven't implemented your own descendant of this class you won't be affected.

The scope of the variables like _controller doesn't allow me to execute the code in main()

Yeah, you'd need a stateful widget for this to work. Overall you'd need to create Observable in the same state object where you manage your ZefyrController instance and whenever you create new controller make sure to dispose and create new observable.

@stemuk
Copy link
Author

stemuk commented Aug 1, 2018

I just tried 0.1.3 out and the two issues can still be reproduced. I think the underlying issue (at least for the list issue) resolves around the preinstalled Android keyboard (GBoard) and its autocorrect feature.
It seems to me that the autocorrected text replaces the originally typed text as soon as the text gets autocorrected by GBoard and -thats my assumption- does not notify the input widget (correctly) that the original text has been replaced, which in turn leads to the error we discussed above.

So the fix either needs to happen on the flutter side of things, or maybe the zefyr editor could handle inputs slightly differently to compensate for GBoards behavior.

As for the 'save to db' issue: The approach you mentioned worked well, thanks a lot for your help 👍

@pulyaevskiy
Copy link
Contributor

Ok, thanks for the update. Will have to dig a bit deeper than.

@pulyaevskiy
Copy link
Contributor

Hey, is it safe to assume this is resolved and can be closed now?

Trying to clean up here a little bit.

@stemuk
Copy link
Author

stemuk commented Oct 12, 2018

Yes, both issues are fixed now. Thanks for solving this!

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

No branches or pull requests

2 participants