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

Implement editor.focus() #714

Closed
fredck opened this issue Dec 13, 2017 · 7 comments · Fixed by #8346 or #8414
Closed

Implement editor.focus() #714

fredck opened this issue Dec 13, 2017 · 7 comments · Fixed by #8346 or #8414
Assignees
Labels
intro Good first ticket. status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@fredck
Copy link
Contributor

fredck commented Dec 13, 2017

🐞 Is this a bug report or feature request? (choose one)

  • Feature request

✅ Expected result

For implementors, I think a common requirement is moving the focus to the editor either on startup or when doing actions outside of it. I was facing this need.

My first approach for it was trying to use the main API entrypoint for it, the editor instance, and call editor.focus(). This is not available.

❎ Actual result

Currently, I'm using the following: editor.element.focus(), which touches the native web API. I assume this worked just because I'm using the ballon editor and my original div is there. I would be even more confused if I would have to do it with other creators that replace my element.

@fredck
Copy link
Contributor Author

fredck commented Dec 13, 2017

Btw, when it comes to the behavior, editor.focus() is not about putting the focus in the editable contents but instead putting the editor in focus. Therefore, if there is any additional UI of the editor active when focus is called, that UI is the one that gets focus.

@Reinmar
Copy link
Member

Reinmar commented Dec 13, 2017

We have the editor.editing.view.focus() method and we didn't need anything more so far. I'm not aware of cases where some other part of editor's UI may need to receive focus from outside – so far, in all our implementation of the editor it was only about focusing the editable in the right way (e.g. the right editable element).

Still, I'd consider implementing editor.focus() (which would call editor.editing.view.focus()) because of discoverability issues. And because specific editors may want to handle this differently.

WDYT? cc @oleq @oskarwrobel @pjasiun

@Reinmar Reinmar added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). status:discussion and removed status:confirmed labels Dec 13, 2017
@fredck
Copy link
Contributor Author

fredck commented Dec 13, 2017

As a clarification, please notice the semantic difference:

  • editor.editing.view.focus() means "make the editing view of the editor focused". It is very specific about which part of the editor is to be focused.
  • editor.focus() means "make the editor focused". I'm not stating which part of the editor is to get focus because I don't care as long as the editor has focus. Ofc, by default it will put the focus in the editing view.

@Reinmar
Copy link
Member

Reinmar commented Dec 13, 2017

I agree. editor.focus() might be used in the future for focusing the right part of the UI. That's why I also agree that it'll be good to implement it. I only meant that we didn't have yet such a use case in which it wouldn't be the editable – perhaps because we didn't work on a11y. But it means that we can't be sure whether such use case will ever appear.

PS. Buttons should still call editor.editing.view.focus() because that's what you expect after e.g. applying bold. You don't want the focus to stay in the toolbar.

@pjasiun
Copy link

pjasiun commented Dec 14, 2017

Still, I'd consider implementing editor.focus() (which would call editor.editing.view.focus()) because of discoverability issues. And because specific editors may want to handle this differently.

Agree. That's not clear at all what to do to focus editor.

@Reinmar
Copy link
Member

Reinmar commented Nov 3, 2020

PS. Buttons should still call editor.editing.view.focus() because that's what you expect after e.g. applying bold. You don't want the focus to stay in the toolbar.

⚠️ Buttons are supposed to focus the editing area. Editor.focus() is a shorthand but it's reasonably likely that it's going to focus also other focusable parts of the editor, depending on what hold the focus recently.

@mlewand
Copy link
Contributor

mlewand commented Nov 3, 2020

As pointed in #714 (comment) the changes in our UI buttons should be reverted. I'll add a PR with a revert for this.

Reinmar added a commit that referenced this issue Nov 5, 2020
Revert: Reverted widespread `editor.focus()` usage back to `editor.editing.view.focus()` form. Closes #714.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intro Good first ticket. status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
6 participants