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

There should be an API to manage the editor balloon #5296

Closed
Reinmar opened this issue Jan 4, 2017 · 10 comments · Fixed by ckeditor/ckeditor5-ui#177
Closed

There should be an API to manage the editor balloon #5296

Reinmar opened this issue Jan 4, 2017 · 10 comments · Fixed by ckeditor/ckeditor5-ui#177
Assignees
Labels
package:ui status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Jan 4, 2017

We encountered this issue in two places already and I think that this is really universal.

There's no functional code yet, so you'll have to imagine this:

  1. Open the image sample.
  2. Focus the image. A balloon appears above the image.
  3. Click the 'alt text' icon.
  4. A new balloon appears... below the image. The toolbar is still visible. We can hide it, but the positions don't match which is a bigger issue

The same happens if you have a contextual toolbar and open a link balloon. Contextual toolbar is linked to the selection focus. The link balloon behaves totally differently. And here we have to hide and show the toolbar manually too.

I'd like to propose that:

  1. We expose a set of two functions to show and hide editor contextual toolbar.
  2. That every feature will operate on that balloon instance. This means that the content of the balloon will be replaced by the feature which opens it.

Now, the functions to show and hide the balloon will be pretty simple – they will accept editor.ui on which they'll store/get the balloon under some symbol. First call to the show() function will create the balloon.

There are at least 2 tricky things which I can see:

  1. Position configuration. How to solve the issue that new balloon appears in a different position than the previous one. I think that it can be done like this – if the toolbar is already visible when you call show() again, then the positions that were previously used are kept. If not, then the passed one will be used (use - you pass positions to show().
  2. There may be some sequences in which the features may lost a balloon or one feature may incorrectly override other feature's balloon. By sequences I mean – you select text, the ctx toolbar appears. Then you click the link button and the link feature calls show() again. Now, selection moves somewhere or sth odd happens, and the ctx toolbar decides that it needs to be rendered again and, even though you're still editing the link, now you see the toolbar instead. But I guess these things we'll need to see and synchronised on our own.

There's one more thing which will be possible after we do this – nice transitions between these panels. We can, with time, work on some size animations or whatever we want. Not easy, but possible.

The other advantage is that we'll have "the close on focus losts" or "reposition on resize" logic in one place.

Finally, the only question which remains is that all this makes sense. That there always will be just one balloon at a time (unless you specifically create your own one, which ofc will be doable, cause this is just a component).

@Reinmar
Copy link
Member Author

Reinmar commented Jan 4, 2017

cc @oleq @pjasiun @szymonkups.

@Reinmar Reinmar changed the title There should be an API to manage the balloon There should be an API to manage the editor balloon Jan 4, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Jan 4, 2017

Now, the functions to show and hide the balloon will be pretty simple – they will accept editor.ui on which they'll store/get the balloon under some symbol. First call to the show() function will create the balloon.

Actually, this can be generalised if we want. The editor.ui may be a context to which the balloon will be attached. You pass another object, and it gets its own balloon.

@szymonkups
Copy link
Contributor

szymonkups commented Jan 4, 2017

What about going back from one balloon to previous one:

  1. Focus the image. An image balloon with toolbar appears above the image.
  2. Click on the alt text change button. Image balloon with toolbar is hidden.
  3. Alt text change balloon appears.
  4. You change alt text and press OK button.
  5. Alt text change balloon is hidden.
  6. How to bring back image balloon?

Just a food for thought: maybe we could have some stack there where we can push/pop content?

@Reinmar
Copy link
Member Author

Reinmar commented Jan 4, 2017

Perhaps show() and hide() could stack their calls. So you close the 2nd balloon and the system shows the 1st one. Or we can solve this by executing show( toolbar ) on something which is caused by changing the link URL (on change?). But this won't work if someone presses the Cancel button, so I guess we need to stack them.

@pjasiun
Copy link

pjasiun commented Jan 4, 2017

What about going back from one balloon to previous one:

Yea, I'm thinking about the same for the contextual toolbar and link toolbar.

Also I'm thinking that this balloon should change its position every time selection change (for instance if we undo selection) so we may need a centralised mechanism of closing it based on the selection change. And maybe opening as well because I can imagine the situation when two plugins (like contextual toolbar and link) want to one balloon at the same time. It could be be called contextual ballon which follow the selection and you can add the listener which say that the panel in needed together with the definition how to positioning it in relation to the selection.

I already like this feature ;)

@Reinmar
Copy link
Member Author

Reinmar commented Jan 4, 2017

I don't like saying that it can be only selection-related. This means that you can have one for the selection and you'd need to create another one if you'd like it to be attached to the toolbar (e.g. the image insert balloon could be implemented this way). IMO, there should be just one balloon at a time for the whole editor UI. Sticking to the selection is just one of its possible features.

@Reinmar
Copy link
Member Author

Reinmar commented Jan 27, 2017

Things are getting more and more complicated when we try to sync the balloons. See e.g. https://github.com/ckeditor/ckeditor5-image/pull/41/files. We definitely need to work on a better API.

@pjasiun
Copy link

pjasiun commented Mar 22, 2017

I think it's high time to fix this issue. I talked with @oskarwrobel and he said he can handle it with some help because the problem is real.

I'm thinking about the API. For sure there should be max one balloon open. It should be the same panel if the context is the same (for instance image can be a context) because it allows us to add animated transitions between panels on the same balloon in the future.

The problem is that we do not know what plugins/panels are installed and what is the source of the panel. For instance: link balloon panel can be open from the top toolbar (then the balloon should be created) or from the contextual toolbar (then the balloon should be reused). I believe that each feature which wants to reuse the contextual balloon should define the "context" and tell to the feature that it should be opened for this context. If the context is the same as previous the panel is reused.

The problem is how to define the context. For images, it could be an image instance, but for the inline toolbar, we have no object we can use as a context.

We can use "selection" as a context, which seems to make sense and fit all cases (for the image it could be a selection around the image), but we should take care to use the same selection in all features which should share the balloon. For instance, it won't work, if one feature use selection around the image with caption and another around the image without it. The same for selection on text and on the link with text. Also what if someone during the collaboration changes the content of the selected text?

On the other hand, we assume that as long the selection does not change caused by the user's action the context is the same.

@pjasiun
Copy link

pjasiun commented Mar 22, 2017

Also, different plugins may have different priorities for its balloons. For instance, contextual toolbar handle any selection as long it is not collapsed, but if it's image what is selected the image contextual balloon should appear instead of the generic one. If the selection is collapsed there should not be a contextual panel, but if it is in the link there should be a link panel. And if link button is clicked the link panel should appear always but if the link button was in the toolbar panel, then it should replace contextual toolbar... yeah, that's tricky.

@pjasiun
Copy link

pjasiun commented Mar 22, 2017

And... the position might be different depending on who initialise the balloon:

The contextual toolbar should appear at the end of selection and the link balloon should appear at the same place if it is opened from that toolbar.

However, if I click on the link right now, I get the balloon in the middle of the link so, if we want to keep this behavior we should let different plugins initialize contextual balloons in a different way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ui status:discussion type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants