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

i/4992: Used the new Collection and View API to add multiple Views at a time #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oleq
Copy link
Member

@oleq oleq commented Oct 21, 2019

Suggested merge commit message (convention)

Internal: Used the new Collection and View API to add multiple Views at a time. See (see ckeditor/ckeditor5#4992).


Additional information

Requires ckeditor/ckeditor5-ui#524.

@coveralls
Copy link

coveralls commented Oct 21, 2019

Coverage Status

Coverage decreased (-0.5%) to 99.51% when pulling faf01b0 on i/4992 into 9db07fd on master.

@oleq oleq added the pr:sub label Oct 21, 2019
@oleq oleq marked this pull request as ready for review October 21, 2019 10:12
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional change in the docs.

* @param {module:ui/colorgrid/colorgrid~ColorDefinition} item
* @param {Number} [index] The position of the item in the collection. The item
* is pushed to the collection when `index` is not specified.
* @param {...(module:ui/colorgrid/colorgrid~ColorDefinition)} items
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at a similar construct in the uitls PR and AFAIC the () is not needed for at least primitives. Maybe in this case we could also drop them?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants