Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

give get() default values #317

Closed
wants to merge 1 commit into from
Closed

give get() default values #317

wants to merge 1 commit into from

Conversation

Eusen
Copy link

@Eusen Eusen commented Dec 6, 2019

The second parameter of the method is used as the default value to minimize code changes
this.editor.config.get( 'blockToolbar', [] );

afterInit() {
        const factory = this.editor.ui.componentFactory;
        const config = this.editor.config.get( 'blockToolbar', [] );

        this.toolbarView.fillFromConfig( config, factory );

        // Hide panel before executing each button in the panel.
        for ( const item of this.toolbarView.items ) {
                item.on( 'execute', () => this._hidePanel( true ), { priority: 'high' } );
        }
}

This should be the least change to the previous code.
@jodator
Copy link
Contributor

jodator commented Dec 16, 2019

As described in other PR I think that for now we're safe with || check. The previous proposal was to add a default value by the existing API:

editor.config.define( 'blockToolbar', [] );

so subsequent calls to editor.config.get( 'blockToolbar' ) will not blow up the editor if no configuration blockToolbar is pased to the editor as in your case.

I'm closing this in favor of: ckeditor/ckeditor5-ui#534 since at the moment we don't want to add additional params to the API as we can achieve the same things otherwise and the config.get() || defaultValue is used already elsewhere.

Thanks for the input :)

@jodator jodator closed this Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants