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

It's almost impossible to click certain buttons in the balloon toolbar (Safari, iOS) #7707

Closed
oleq opened this issue Jul 24, 2020 · 14 comments · Fixed by #10736
Closed

It's almost impossible to click certain buttons in the balloon toolbar (Safari, iOS) #7707

oleq opened this issue Jul 24, 2020 · 14 comments · Fixed by #10736
Assignees
Labels
browser:safari domain:mobile This issues reports a problem related to a mobile environment. domain:ui/ux This issue reports a problem related to UI or UX. package:build-balloon package:build-balloon-block squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Jul 24, 2020

📝 Provide detailed reproduction steps (if any)

  1. Open the balloon/balloon block example.
  2. Select some text.
  3. Try clicking buttons in the toolbar that horizontally align with the visual selection focus handler, like Bold in the example above (it could be a different button depending on the geometry):

✔️ Expected result

All toolbar buttons should be clickable.

❌ Actual result

For some unknown reason buttons next to the selection handler cannot be clicked as if something covered them (native iOS behavior/optimization next to selection handles?).

📃 Other details

  • Browser: Safari
  • OS: iOS
  • CKEditor version: master

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. domain:ui/ux This issue reports a problem related to UI or UX. browser:safari package:build-balloon package:build-balloon-block domain:mobile This issues reports a problem related to a mobile environment. labels Jul 24, 2020
@oleq
Copy link
Member Author

oleq commented Jul 24, 2020

An afterthought/rough idea: what if we made the balloon triangle bigger to increase the distance between the selection handle and the buttons?

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Jul 28, 2020
@Reinmar
Copy link
Member

Reinmar commented Jul 28, 2020

We can also just move this balloon ~5px-10px lower/higher on iOS.

@Reinmar
Copy link
Member

Reinmar commented Jul 28, 2020

@oleq said that we'd have to move it by ~20px to make a difference. In this case, should we hide the arrow?

@Reinmar Reinmar added this to the nice-to-have milestone Aug 3, 2020
@Reinmar
Copy link
Member

Reinmar commented Aug 18, 2021

Let's do this after #9892.

@Reinmar
Copy link
Member

Reinmar commented Aug 18, 2021

Scope:

  • The first option to try: Just move the balloon away from the text (on mobile Safari only)
  • Optional: Make the arrow bigger if the above-mentioned change will make the balloon position look bad. Or, even simpler: hide the arrow (thanks to the new option introduced in Table toolbar does not respect viewportTopOffset configuration #9892).
  • Add env.isiOS
  • Proposed implementation approach: Increase the size of the rect(s) to which we attach the balloon. This will ensure that the shift of the balloon happens before we calculate whether it fits in the viewport and hence the balloon will not be shifted out of the viewport.
  •  The tricky bit may be where to locate the code that will ensure that the shift happens only when the balloon is positioned to the selection. We must not change the positioning of balloons for objects like e.g. image.
  • Verify how it works for smaller font sizes where Safari will zoom-in on the text selection.

@Reinmar Reinmar modified the milestones: nice-to-have, iteration 47 Aug 30, 2021
@Reinmar Reinmar removed this from the iteration 47 milestone Sep 13, 2021
@oleq
Copy link
Member Author

oleq commented Oct 18, 2021

Interesting fact: the distance around the selection handle changes depending on the zoom level of the browser

Zoomed out
Screenshot 2021-10-18 at 16 20 34

Zoomed in
Screenshot 2021-10-18 at 16 19 49

@oleq oleq self-assigned this Oct 18, 2021
@dawidurbanski
Copy link
Contributor

@oleq interesting... how did you determine the size of this area? By trial and error clicking around?

@oleq
Copy link
Member Author

oleq commented Oct 19, 2021

Yep. I'll post my PoC to address this issue later today.

@oleq
Copy link
Member Author

oleq commented Oct 21, 2021

PoC branch #10736.

Findings

How it looks when zoomed out 

How it looks when zoomed in

I had to restrict the number of available positions because some of them look weird when the balloon is far away and arow-less. I left only some basic ones (centered and extremes), dropped "middle" ones etc. This will make the toolbar more likely to go off the screen, so it's a sort of trade-off

Other than this, I tried to limit the fix to the BalloonToolbar only. But because our ContextualBalloon re-uses previous positions, for instance, when entering the link balloon from the balloon toolbar, the new look&behavior will be (re-)used and when the link UI is open directly by clicking a link, the old one will be used. Maybe we should apply the new look to all balloons? It's a tough decision.

@Reinmar
Copy link
Member

Reinmar commented Oct 22, 2021

Out of curiosity – does the viewport API tell us about the position of the native balloon?

How does it work now that our balloon is placed on the opposite side of the selection than the native one? Is it intentional or somehow accidental?

My two other random thoughts:

  • Visually... I'd say that our balloon feels too far away from the selection. It looks slightly odd. I know it was the point of this ticket, however, we didn't know at the beginning how far we'll have to move it... and it's far.
  • Maybe our toolbar could be placed above the native one (if we can reliably figure out where it is? In other words, maybe we could try sticking it to the native one?

@Reinmar
Copy link
Member

Reinmar commented Oct 22, 2021

  • Visually... I'd say that our balloon feels too far away from the selection. It looks slightly odd. I know it was the point of this ticket, however, we didn't know at the beginning how far we'll have to move it... and it's far.

BTW, did you move the balloon completely beyond the range of the selection "pin" clickable area? Maybe we don't have to move it completely outside, but just a bit further so the overlapping area is smaller? Thus, we'd reduce a bit the distance and maybe it'd look less odd (to me :P).

@oleq
Copy link
Member Author

oleq commented Oct 22, 2021

BTW, did you move the balloon completely beyond the range of the selection "pin" clickable area? Maybe we don't have to move it completely outside, but just a bit further so the overlapping area is smaller? Thus, we'd reduce a bit the distance and maybe it'd look less odd (to me :P).

I moved it far enough for the entire button to be clickable. And I agree, this looks weird.

@oleq
Copy link
Member Author

oleq commented Oct 22, 2021

I moved it far enough for the entire button to be clickable. And I agree, this looks weird.

Or maybe it's not about the distance but about the lack of the triangle (arrow) that would give it context.

@oleq
Copy link
Member Author

oleq commented Oct 28, 2021

Awaiting feedback from the team on #10736.

oleq added a commit that referenced this issue Nov 18, 2021
Fix (ui): Changed the look and position of the `BalloonToolbar` in Safari on iOS to avoid a clash with native text selection handles. Closes #7707.

Feature (utils): Introduced `env.isiOS` for detection of user agents running in iOS (see #7707).

Internal (utils,minimap): Moved the `RectDrawer` dev helper from `ckeditor5-minimap` to `ckeditor5-utils`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:safari domain:mobile This issues reports a problem related to a mobile environment. domain:ui/ux This issue reports a problem related to UI or UX. package:build-balloon package:build-balloon-block squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants