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

i/5692: Added documentation for the config.toolbar.shouldNotGroupWhenFull option #201

Merged
merged 2 commits into from
Nov 20, 2019
Merged

i/5692: Added documentation for the config.toolbar.shouldNotGroupWhenFull option #201

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae8f9f3
Docs: Added documentation for the config.toolbar.shouldGroupWhenFull …
oleq Nov 18, 2019
ee2b5ee
Renamed config.toolbar.shouldGroupWhenFull to config.toolbar.shouldNo…
oleq Nov 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/editor/editorconfig.jsdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@
* toolbar: {
* items: [ 'bold', 'italic', '|', 'undo', 'redo' ],
*
* viewportTopOffset: 30
* viewportTopOffset: 30,
*
* shouldGroupWhenFull: true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* shouldGroupWhenFull: true
* shouldGroupWhenFull: false

No one will be setting it to true as this is the default.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I'd rather rename it to shouldNotGroupWhenFull to avoid a flag which is default by true. Then you'll also have less issues with handling the default in the other two PRs.

* }
* };
*
Expand All @@ -162,6 +164,11 @@
* * **`toolbar.viewportTopOffset`** – The offset (in pixels) from the top of the viewport used when positioning a sticky toolbar.
* Useful when a page with which the editor is being integrated has some other sticky or fixed elements
* (e.g. the top menu). Thanks to setting the toolbar offset the toolbar will not be positioned underneath or above the page's UI.
* * **`toolbar.shouldGroupWhenFull`** – When `true` (default), the toolbar will group its items that
Copy link
Member Author

Choose a reason for hiding this comment

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

This documentation entry is growing (items, viewportTopOffset and now shouldGroupWhenFull). I know it's convenient for developers to keep it in a single place (module:core/editor/editorconfig~EditorConfig#toolbar) but if we get more options like this, I think it's time to define some @interface ToolbarConfig and put the detailed documentation there.

Copy link
Member

Choose a reason for hiding this comment

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

As long as those options are fairly simple (boolean, number, simple array), I'd keep them locally here because, as you mentioned, that more convenient to read. But I agree that at some point we may need to extract it. But then we can keep a lot of information here locally, and have details in the interface.

* would normally wrap to the next line when there is not enough space to display them in a single row.
*
* **Note**: For now this option works for {@link module:editor-classic/classiceditor~ClassicEditor} and
* {@link module:editor-decoupled/decouplededitor~DecoupledEditor} only.
*
* @member {Array.<String>|Object} module:core/editor/editorconfig~EditorConfig#toolbar
*/
Expand Down