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

Incorrect toggling of full-screen mode when using annotation tools #178

Open
dstillman opened this issue Oct 29, 2024 · 6 comments
Open
Labels

Comments

@dstillman
Copy link
Member

dstillman commented Oct 29, 2024

And then tapping on the page toggles it again. I'm actually having trouble even reliably creating an annotation. For example, in non-full-screen mode:

  1. Tap the toolbar button to show the annotation toolbar.
  2. Tap T. The reader incorrectly switches into full-screen mode.
  3. Tap the page background. The reader incorrectly switches out of full-screen mode and doesn't create an annotation, and the T is deactivated.
  4. Tap the page background again. The reader switches back into full-screen mode. The T doesn't show as selected.
  5. Tap the page background again. A text annotation is finally created, despite T not having been showing as selected.
@dstillman
Copy link
Member Author

dstillman commented Oct 29, 2024

And then a bonus bug: you have to tap again on the text annotation field to select it and show the keyboard.

It should be three taps: once to show the annotation toolbar, once on T, and once on the page. You should then be able to type.

Dima-Android added a commit that referenced this issue Oct 30, 2024
@Dima-Android
Copy link
Collaborator

I’ve completely reworked the logic of showing/hiding top and bottom bars. We used to rely on PSPDFKIT to tell us when to hide our top bar. We used to always sync top bar visibility with the integrated into the PSPDFKIT’s bottom bar (thumbnail grid) and for some reason whenever annotation tools were selected/deselected it always triggered UI visibility callback.
Now we control visibility of top/bottom bars in manual mode.
When I looked closer at how iOS is doing it I realized there are cases when the top bar is visible while the bottom bar is hidden. For example this happens during scrolling. Also iOS does not do anything to top/bottom bars when interacting with any of the annotation tools.
Accounting for all of this should help with the jumpy UI you described.

T button getting deselected after you place the TextBox and text field inside of it getting auto-focused is also what happens on iOS. I think it's the behavior of PSPDFKIT to deselect the currently selected tool right after the empty TextBox is placed and we are notified of this after the fact.
Not completely sure whether the TextBox field not getting focused and keyboard not appearing would be fixed by these changes, but there is a chance that it will since some of the jumpiness was eliminated. At least testing on my devices shows that the field is getting focused now.

Please test it and see if it works better and whether other adjustments are needed.

@dstillman
Copy link
Member Author

Well, the good news is that worked once — I tapped T, I tapped the page, the textbox appeared, and the keyboard popped up.

The bad news is that now I'm tapping T and tapping the page and the T gets selected and nothing shows up, and then if I tap the page again…things get weird. Sometimes a textbox shows up, sometimes full-screen mode toggles, sometimes a textbox shows up in a previous location, sometimes it maybe shows up where I tapped? Oh, and looking at the sidebar, it seems like it's just creating empty annotations all around the page. It's definitely not usable.

If I force-quit the app, it works properly again once, and then stops working properly after that.

@Dima-Android
Copy link
Collaborator

Record a video, I can't reproduce it.

@dstillman
Copy link
Member Author

dstillman commented Oct 31, 2024

This doesn't show taps, but you can see when the T gets deselected and nothing immediately shows up that I was tapping on the page. Other taps are selecting empty text annotations from previous attempts. (I can't get rid of them due to #181.)

Among other things, those empty ones shouldn't exist at all — if there's no text and you tap off an annotation, it shouldn't be created or it should be deleted if it already exists. But the initial problem is that tapping is only working properly, with visible textbox and keyboard, on the first try.

Text.Annotations.mp4

Dima-Android added a commit that referenced this issue Nov 5, 2024
…ation tools #178

Deleting empty text annotations on defocus.

Upping versionCode to 114
@Dima-Android
Copy link
Collaborator

On my phone and emulators it’s a very intermittent bug, I had to super-quickly create about 30 text annotations to reproduce this.
I think I’ve found the problem - a race condition that happens when two things happen:
onAnnotationAdded PSPDFKIT callback is triggered and we add Annotation to a database
Another PSPDFKIT callback OnAnnotationSelected triggers our code related to processing the selection of annotation and as part of this we need to call selectedAnnotation function which is supposed to find this annotation in the database by key.
Sometimes N2 is called before the N1 is finished, hence the selectedAnnotation returns null and this makes our code wrongly assume that the user is doing a deselection and so our code calls PSPDFKIT’s clearSelectedAnnotations() which leads to what you see - empty annotation is created, then immediately deselected and hence keyboard is not appearing..
The solution is to make sure those two processes run on the same thread hence avoiding the race condition.

But I am not sure if I got it right, so please test again.

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

No branches or pull requests

2 participants