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

Editor mobile touch detection has degraded #234

Closed
bjhess opened this issue Dec 5, 2024 · 15 comments · Fixed by #240
Closed

Editor mobile touch detection has degraded #234

bjhess opened this issue Dec 5, 2024 · 15 comments · Fixed by #240

Comments

@bjhess
Copy link
Contributor

bjhess commented Dec 5, 2024

I'll try to explain this and then share some videos if I can get them captured.

Prior to our upgraded from Rhino 0.10.2 to 0.14.1, I always felt that tapping into the editor on mobile worked well. (In my experience with our editor on the Pika app.) Now it has become frustrating, and I think this frustration also exists in the default Rhino editor here. So I figured a bug report!

Now when I tap to a specific place in the editor, if the cursor was prior at the top of the text being edited, the screen will snap up to that cursor location rather than setting the cursor to the location where I just tapped. In Pika right now I sometimes have to tap 3 times to get the cursor where I need it to be, and that's often after also scrolling. (Though I can only seem to recreate a 2-taps experience right now.)

I often edit a few long pages on my site through the editor in Pika, so this change is quite noticeable. It worked smooth as butter before.

Others experiencing this as a regression as well?

Attempted video evidence…part of the prerequisite here is that the cursor in the editor is at the top of the editor before the interaction. Rhino:

https://share.cleanshot.com/g5PGG1gH

Pika:

https://share.cleanshot.com/1lqq4f2g

(Sorry I don't have highlights in the video that show where I'm tapping. Hopefully it can be intuited with a couple watches. 😬 )

@KonnorRogers
Copy link
Owner

KonnorRogers commented Dec 5, 2024

🤔

im also noticing the mobile editor keeps trying to highlight instead of use the cursor.

The 2 big changes were inline-code and the selection plugin.

If you don't mind, could you give this a try:

rhinoEditor.starterKitOptions = {
  ...rhinoEditor.starterKitOptions,
  rhinoCodemarkPlugin: false,
  rhinoSelection: false,
}

@KonnorRogers
Copy link
Owner

If it makes it easier:

https://codepen.io/paramagicdev/pen/raBxJKV

@KonnorRogers
Copy link
Owner

🤔 I even disabled bubble-menu here.

I may have to do some diffing to find out what changed between those releases, hopefully its not an issue with the core prosemirror / tiptap libraries.

https://stackblitz.com/edit/vue-35bwhe?file=src%2FApp.vue

^ For posterity, theres TipTap 3.0, id be curious if you see the same behavior there.

@bjhess
Copy link
Contributor Author

bjhess commented Dec 5, 2024

https://codepen.io/paramagicdev/pen/raBxJKV
https://stackblitz.com/edit/vue-35bwhe?file=src%2FApp.vue

Both of these seem to work as I'd expect. (A little fiddly to tap around on those pages with the editors involved, but I think I confirmed it working well.)

@KonnorRogers
Copy link
Owner

@bjhess okay that helps a lot.

im pretty sure this has to do with the injected span for link selections 🤔 ill have to do some testing on it to fix it. If its that bad, you should be safe to disable it (I would double check and confirm disabling only that one fixes it) there was some trickery for safari that may need revisiting.

@bjhess
Copy link
Contributor Author

bjhess commented Dec 5, 2024

Thanks, @KonnorRogers!

@KonnorRogers
Copy link
Owner

@bjhess Do you mind enabling the plugin and try adding this to your CSS:

  .rhino-insertion-placeholder {
    pointer-events: none;
    -webkit-user-select: none;
  }

And letting me know if you still see the cursor issues?

@bjhess
Copy link
Contributor Author

bjhess commented Dec 9, 2024

I don't have any reliable way to run our app locally and access it via mobile, so I YOLO deployed the CSS and didn't see any difference in behavior.

Did you see any difference in a CodePen?

@KonnorRogers
Copy link
Owner

@bjhess ahhh okay, I thought I did, but had been playing with it for quite a while so may have just been losing my mind.

I may just disable this for now until i can determine a better fix.

@bjhess
Copy link
Contributor Author

bjhess commented Dec 9, 2024

I may just disable this for now until i can determine a better fix.

Do you plan to disable both rhinoCodemarkPlugin and rhinoSelection?

@KonnorRogers
Copy link
Owner

Nope. I dont think rhinoCodemarkPlugin is causing any issues.

rhinoSelection will still be in, but i'll disable the injected span for when you don't have anything selected.

// Show a fake cursor.
let widget = document.createElement("placeholder");
// TODO: Make this configurable.
widget.setAttribute("class", "rhino-insertion-placeholder");
widget.setAttribute("readonly", "");
widget.setAttribute("contenteditable", "false");
deco = Decoration.widget(selection.to, widget, {});

These lines seem to be the problem.

@bjhess
Copy link
Contributor Author

bjhess commented Dec 9, 2024

Sounds good. Hopefully it'll be reasonably easy to figure out!

@KonnorRogers
Copy link
Owner

0.14.3 removed the insertion span. Decorator for showing selected text for links should still be there. Let me know if you're still having problems.

@bjhess
Copy link
Contributor Author

bjhess commented Dec 11, 2024

Things are looking good now, @KonnorRogers!

@KonnorRogers
Copy link
Owner

Good to hear, and also good to know for the future should I look to add any other decorations 🙈

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 a pull request may close this issue.

2 participants