API for changing selected item for rich combo based plugins

[jacekbogdanski]

Type of report

Feature request

Provide description of the new feature

As we already started working on #3306 we will also need API allowing to change combo box selected item. We agreed together with @f1ames and @msamsel that the feature should be possible generic and easy to use.

Current proposition is to extend rich combo API with additional select method, accepting filtering function (based mostly on find algorithm) allowing to choose correct style definition. As an example, font plugin resulting with rich combo component:

// First match will decide about selected item.
editor.ui.get( 'Font' ).select( function( definition ) {
  return definition.style[ 'font-family' ] === 'Arial';
}

As some plugins like font uses more complex object to resolve correct content style, select method callback should accept CKEDITOR.style.getDefinition() object, extensively used by rich combo related plugins to set content style.

It will also require providing some optional method to implement required definition mapping to avoid breaking changes with other rich combo plugins. It may make sense to fully delegate select filtering callback into plugin implementing rich combo to avoid rich combo and CKEDITOR.style dependency or just by providing simple ID to CKEDITOR.style mapping. Not totally sure which method would be better here, I would leave it to developer decision which seems more verbose / easier to implement.

[msamsel]

@jacekbogdanski, @f1ames.
I think it could be nice idea to return true or false value. However, I wonder what information should be indicated by this value. I see 2 possibilities:

1. Information that value was set.
2. Information that value is a new one and was changed.

For example.
If you have already set in richcombo "Tahoma" font, and you perform a select method which is going to select "Tahoma", then what result would you expect on the output?

• true - because was sucesfull set the Tahoma font (even though this value was already set up)
• false - because there wasn't changed anything in the richcombo.

What can be more useful in this case from your perspective?

[jacekbogdanski]

I'm thinking about usage of boolean return from select method as it seems for me more like fire and forget. But it's not that easy as the filter method may not return with any truthy value. Overall it seems like implementation error as it should always match something. Maybe it should just return false if nothing has been returned by filter callback otherwise true? We could also go more pythonic version and just throw an error which seems also good for me.

On the other hand, I'm not sure if we should inform successful/failure value change - treating this method as a simple setter may simplify API. Do you see some use cases where such information would be useful?

[msamsel]

Maybe it should just return false if nothing has been returned by filter callback otherwise true?

Yes, this approach seems a little bit better for me as well. There is also another case when matching might get wrong. We base that richcombo preserve mapping on its side, in case that plugin's author doesn't provide the mapping then also might be no match.

The error also seems reasonable for me.

Do you see some use cases where such information would be useful?

I wonder if there might appear some situation when the browser has some quirks or there is 3rd party plugin which interferes with styles. So there might be needed to know if there was a success of setting richcombo or not.

However, if you don't see currently any application in your case, then removing it and API simplification seems like the best option for me. It can be always added in the future.

[f1ames]

I mostly agree with @jacekbogdanski, that using select() as a simple setter will be the best idea. The one using it should be aware of all drawbacks and use it correctly. And if we don't see any usage for return value we may skip it - TBH I'm not sure how useful it can be... maybe apart from cases when if specific value wasn't set, then set default one. OTOH one may simple call getValue() on richcombo after calling select() to see if value was set correctly.

So I'm for simplifying it with void return value.