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

removeFromToolbar in config #7945

Closed
piernik opened this issue Aug 26, 2020 · 5 comments · Fixed by #8649
Closed

removeFromToolbar in config #7945

piernik opened this issue Aug 26, 2020 · 5 comments · Fixed by #8649
Assignees
Labels
domain:dx This issue reports a developer experience problem or possible improvement. intro Good first ticket. package:ui type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@piernik
Copy link

piernik commented Aug 26, 2020

I have custom build with all tools and buttons that I need. But In some cases I don't want link button to be shown. I need option removeFromToolbar for that. Otherwise I have to rewrite my whole toolbar config, which can change in time.

BTW. I think that option removePlugins is not working. I'm adding

removePlugins: [
   'Bold'
],

and bold is working as always. But still (as in docs) removePlugins does not remove corresponding buttons.

@piernik piernik added the type:feature This issue reports a feature request (an idea for a new functionality or a missing option). label Aug 26, 2020
@Reinmar
Copy link
Member

Reinmar commented Oct 5, 2020

Interesting proposal. I stumbled upon this problem many times too.

@Reinmar Reinmar added domain:dx This issue reports a developer experience problem or possible improvement. package:ui squad:dx intro Good first ticket. labels Oct 5, 2020
@Reinmar Reinmar modified the milestones: next, backlog Oct 5, 2020
@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2020

Expected usage:

ClassicEditor.create( {
	toolbar: {
		removeItems: [ 'bold' ]
	}
} )

@Reinmar
Copy link
Member

Reinmar commented Dec 11, 2020

Edge case: there were | used like this:

	toolbar: {
		items: [
			'heading',
			'|',
			'bold',
			'italic',
			'link',
			'bulletedList',
			'numberedList',
			'|',
			'indent',
			'outdent',
			'|',
			'imageUpload',
			'blockQuote',
			'insertTable',
			'mediaEmbed',
			'undo',
			'redo'
		]
	},

And you defined removeItems: [ 'indent', 'outdent' ] so after that you'd end up with two separators next to each other.

Perhaps we can relatively easily normalize this.

It seems that all this code can be put in ToolbarView.fillFromConfig.

@maxbarnas
Copy link
Contributor

maxbarnas commented Dec 11, 2020

Since ToolbarView.fillFromConfig accepts items directly, and not the entire toolbar config, I have to change both the fillFromConfig signature and the calls in the following packages:

  • ui,
  • widget,
  • internal,
  • and editor bundles.

@maxbarnas
Copy link
Contributor

We decided to move forward with the change. ToolbarView.fillFromConfig will handle both an array and object passed as parameters. In the first case, it will behave the same way as now, in the latter, it will treat the object as the entire toolbar config instead of just items.

CC @oleq

@AnnaTomanek AnnaTomanek modified the milestones: backlog, iteration 39 Dec 14, 2020
@maxbarnas maxbarnas added the squad:core Issue to be handled by the Core team. label Dec 21, 2020
@oleq oleq removed the squad:core Issue to be handled by the Core team. label Dec 21, 2020
Reinmar added a commit that referenced this issue Dec 22, 2020
Feature (ui): Items baked into editor bundles can be removed from the toolbar by using `config.toolbar.removeItems`. Closes #7945.

MINOR BREAKING CHANGE (ui): Configuration passed to `ToolbarView.fillFromConfig()` will be stripped off of leading, trailing, and duplicated separators ('|' and '-').
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dx This issue reports a developer experience problem or possible improvement. intro Good first ticket. package:ui 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