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

BalloonPanelView should not be focusable #5344

Closed
oleq opened this issue Apr 19, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-ui#213
Closed

BalloonPanelView should not be focusable #5344

oleq opened this issue Apr 19, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-ui#213
Assignees
Labels
package:ui type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@oleq
Copy link
Member

oleq commented Apr 19, 2017

kapture 2017-04-19 at 9 09 23

Because of that we end up with problems like https://github.com/ckeditor/ckeditor5-link/issues/90. It's probably just a matter of using the @mixin ck-unselectable in SASS.

@Reinmar
Copy link
Member

Reinmar commented Apr 19, 2017

Isn't that CSS supported by Chrome only?

@oleq
Copy link
Member Author

oleq commented Apr 19, 2017

Nope. It's supported by almost every major web browser.

@Reinmar
Copy link
Member

Reinmar commented Apr 19, 2017

And why is the balloon focusable in the first place?

@oleq
Copy link
Member Author

oleq commented Apr 19, 2017

Most certainly because the author (me) has never considered this thing becoming a problem and obviously was wrong ;-)

@Reinmar
Copy link
Member

Reinmar commented Apr 19, 2017

There must've been some reason. I even recall there was something... But I don't remember what and whether this is still any relevant. Hopefully not... but I remember that CKE4's dialogs were also focusable.

@oleq
Copy link
Member Author

oleq commented Apr 19, 2017

but I remember that CKE4's dialogs were also focusable.

They aren't. Or at least, they don't look like they accept focus.

@Reinmar
Copy link
Member

Reinmar commented Apr 19, 2017

They aren't. Or at least, they don't look like they accept focus.

That's odd. I was nearly sure they were. So... I'll better stop confusing you :D

@oskarwrobel
Copy link
Contributor

oskarwrobel commented Apr 19, 2017

// Make this element `focusable` to be available for adding to FocusTracker.
tabindex: -1

https://github.com/ckeditor/ckeditor5-ui/blob/master/src/panel/balloon/balloonpanelview.js#L131

@oleq
Copy link
Member Author

oleq commented Apr 19, 2017

Maybe we don't need it anymore, hard to tell.

@oleq oleq self-assigned this Apr 20, 2017
oskarwrobel referenced this issue in ckeditor/ckeditor5-ui Apr 24, 2017
T/206a: BalloonPanelView should not be focusable
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the iteration 10 milestone Oct 9, 2019
@mlewand mlewand added type:bug This issue reports a buggy (incorrect) behavior. package:ui labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ui type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
4 participants