From 43ecf9992aa3dc64bec1d9ed02a39168d84200f1 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 13 Apr 2023 20:14:18 +0200 Subject: [PATCH 01/17] Document list basic integration with styles dropdown. --- .../src/generalhtmlsupport.ts | 6 + .../src/documentlist/documentlistutils.ts | 14 ++- packages/ckeditor5-style/package.json | 3 + packages/ckeditor5-style/src/stylecommand.ts | 57 +++++++++- .../tests/manual/styledropdown.html | 107 ++++++++++++++++++ .../tests/manual/styledropdown.js | 56 +++++++-- 6 files changed, 229 insertions(+), 14 deletions(-) diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts index b372ce34e89..2ad8787edc1 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts @@ -80,6 +80,12 @@ export default class GeneralHtmlSupport extends Plugin { * @param viewElementName A view element name. */ private getGhsAttributeNameForElement( viewElementName: string ): string { + if ( viewElementName == 'ol' || viewElementName == 'ul' ) { + return 'htmlListAttributes'; + } else if ( viewElementName == 'li' ) { + return 'htmlLiAttributes'; + } + const dataSchema = this.editor.plugins.get( 'DataSchema' ); const definitions = Array.from( dataSchema.getDefinitionsForView( viewElementName, false ) ); diff --git a/packages/ckeditor5-list/src/documentlist/documentlistutils.ts b/packages/ckeditor5-list/src/documentlist/documentlistutils.ts index c2e1af8ac3a..cc7b4493760 100644 --- a/packages/ckeditor5-list/src/documentlist/documentlistutils.ts +++ b/packages/ckeditor5-list/src/documentlist/documentlistutils.ts @@ -11,7 +11,12 @@ import type { Element, Node } from 'ckeditor5/src/engine'; import type { ArrayOrItem } from 'ckeditor5/src/utils'; import { Plugin } from 'ckeditor5/src/core'; -import { expandListBlocksToCompleteList, isFirstBlockOfListItem, isListItemBlock } from './utils/model'; +import { + expandListBlocksToCompleteItems, + expandListBlocksToCompleteList, + isFirstBlockOfListItem, + isListItemBlock +} from './utils/model'; /** * A set of helpers related to document lists. @@ -50,5 +55,12 @@ export default class DocumentListUtils extends Plugin { public isListItemBlock( node: Node ): boolean { return isListItemBlock( node ); } + + /** + * TODO + */ + public expandListBlocksToCompleteItems( blocks: ArrayOrItem, options: { withNested?: boolean } = {} ): Array { + return expandListBlocksToCompleteItems( blocks, options ); + } } diff --git a/packages/ckeditor5-style/package.json b/packages/ckeditor5-style/package.json index ca337dbf626..9039f7a6c2b 100644 --- a/packages/ckeditor5-style/package.json +++ b/packages/ckeditor5-style/package.json @@ -17,6 +17,7 @@ }, "devDependencies": { "@ckeditor/ckeditor5-alignment": "^37.0.1", + "@ckeditor/ckeditor5-autoformat": "^37.0.1", "@ckeditor/ckeditor5-basic-styles": "^37.0.1", "@ckeditor/ckeditor5-block-quote": "^37.0.1", "@ckeditor/ckeditor5-cloud-services": "^37.0.1", @@ -26,6 +27,7 @@ "@ckeditor/ckeditor5-easy-image": "^37.0.1", "@ckeditor/ckeditor5-editor-classic": "^37.0.1", "@ckeditor/ckeditor5-engine": "^37.0.1", + "@ckeditor/ckeditor5-essentials": "^37.0.1", "@ckeditor/ckeditor5-font": "^37.0.1", "@ckeditor/ckeditor5-heading": "^37.0.1", "@ckeditor/ckeditor5-highlight": "^37.0.1", @@ -37,6 +39,7 @@ "@ckeditor/ckeditor5-language": "^37.0.1", "@ckeditor/ckeditor5-link": "^37.0.1", "@ckeditor/ckeditor5-list": "^37.0.1", + "@ckeditor/ckeditor5-media-embed": "^37.0.1", "@ckeditor/ckeditor5-mention": "^37.0.1", "@ckeditor/ckeditor5-page-break": "^37.0.1", "@ckeditor/ckeditor5-paragraph": "^37.0.1", diff --git a/packages/ckeditor5-style/src/stylecommand.ts b/packages/ckeditor5-style/src/stylecommand.ts index 04f4c0c8c60..c7b3b2a5590 100644 --- a/packages/ckeditor5-style/src/stylecommand.ts +++ b/packages/ckeditor5-style/src/stylecommand.ts @@ -11,6 +11,7 @@ import type { Element, Schema } from 'ckeditor5/src/engine'; import { Command, type Editor } from 'ckeditor5/src/core'; import { logWarning, first } from 'ckeditor5/src/utils'; import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; +import type { DocumentListUtils } from '@ckeditor/ckeditor5-list'; import { isObject } from 'lodash-es'; import type { BlockStyleDefinition, InlineStyleDefinition, NormalizedStyleDefinitions } from './styleutils'; @@ -67,6 +68,8 @@ export default class StyleCommand extends Command { public override refresh(): void { const model = this.editor.model; const selection = model.document.selection; + const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? + this.editor.plugins.get( 'DocumentListUtils' ) : null; const value = new Set(); const enabledStyles = new Set(); @@ -101,20 +104,43 @@ export default class StyleCommand extends Command { break; } - if ( !model.schema.checkAttribute( block, 'htmlAttributes' ) ) { + if ( + !model.schema.checkAttribute( block, 'htmlAttributes' ) && + !model.schema.checkAttribute( block, 'htmlListAttributes' ) && + !model.schema.checkAttribute( block, 'htmlLiAttributes' ) + ) { continue; } for ( const definition of this._styleDefinitions.block ) { // Check if this block style is enabled. - if ( !definition.modelElements.includes( block.name ) ) { + if ( documentListUtils && documentListUtils.isListItemBlock( block ) ) { + if ( definition.element == 'ol' || definition.element == 'ul' ) { + const viewElementName = block.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; + + if ( definition.element != viewElementName ) { + continue; + } + } else if ( definition.element != 'li' && !definition.modelElements.includes( block.name ) ) { + continue; + } + } else if ( !definition.modelElements.includes( block.name ) ) { continue; } enabledStyles.add( definition.name ); // Check if this block style is active. - const ghsAttributeValue = block.getAttribute( 'htmlAttributes' ); + let attributeName = 'htmlAttributes'; + + if ( + [ 'ol', 'ul', 'li' ].includes( definition.element ) && + documentListUtils && documentListUtils.isListItemBlock( block ) + ) { + attributeName = definition.element == 'li' ? 'htmlLiAttributes' : 'htmlListAttributes'; + } + + const ghsAttributeValue = block.getAttribute( attributeName ); if ( hasAllClasses( ghsAttributeValue, definition.classes ) ) { value.add( definition.name ); @@ -181,11 +207,14 @@ export default class StyleCommand extends Command { const shouldAddStyle = forceValue === undefined ? !this.value.includes( definition.name ) : forceValue; + const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? + this.editor.plugins.get( 'DocumentListUtils' ) : null; + model.change( () => { let selectables; if ( isBlockStyleDefinition( definition ) ) { - selectables = getAffectedBlocks( selection.getSelectedBlocks(), definition.modelElements, model.schema ); + selectables = getAffectedBlocks( selection.getSelectedBlocks(), definition.modelElements, model.schema, documentListUtils ); } else { selectables = [ selection ]; } @@ -243,7 +272,8 @@ function hasAllClasses( ghsAttributeValue: unknown, classes: Array ): bo function getAffectedBlocks( selectedBlocks: IterableIterator, elementNames: Array, - schema: Schema + schema: Schema, + documentListUtils: DocumentListUtils | null ): Set { const blocks = new Set(); @@ -255,6 +285,23 @@ function getAffectedBlocks( break; } + if ( documentListUtils && documentListUtils.isListItemBlock( block ) ) { + // TODO make some precise option for this + if ( elementNames.includes( 'htmlOl' ) || elementNames.includes( 'htmlUl' ) ) { + for ( const element of documentListUtils.expandListBlocksToCompleteList( block ) ) { + blocks.add( element ); + } + + break; + } else if ( elementNames.includes( 'htmlLi' ) ) { + for ( const element of documentListUtils.expandListBlocksToCompleteItems( block, { withNested: false } ) ) { + blocks.add( element ); + } + + break; + } + } + if ( elementNames.includes( block.name ) ) { blocks.add( block ); diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.html b/packages/ckeditor5-style/tests/manual/styledropdown.html index 5a7b8e0943d..8285799402f 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.html +++ b/packages/ckeditor5-style/tests/manual/styledropdown.html @@ -109,12 +109,119 @@ color: hsl(0, 0%, 40%); } + .ck.ck-content ol.fancy-list > li { + counter-increment: list; + list-style: none; + margin-bottom: 2.5em; + padding-left: 70px; + } + + .ck.ck-content ol.fancy-list > li::before { + font-weight: bold; + font-size: 2em; + content: counter(list, decimal); + width: 50px; + height: 50px; + float: left; + margin: 5px 20px 0 -78px; + color: #fff; + background: #000; + clip-path: polygon(0 0, 100% 0, 100% 65%, 65% 100%, 0 100%); + display: inline-flex; + align-items: center; + justify-content: center; + shape-outside: ellipse(); + font-family: 'Anton', cursive; + } + + .ck.ck-content ol.fancy-list[style*="decimal"] > li::before { + content: counter(list, decimal); + } + + .ck.ck-content ol.fancy-list[style*="decimal-leading-zero"] > li::before { + content: counter(list, decimal-leading-zero); + } + + .ck.ck-content ol.fancy-list[style*="lower-roman"] > li::before { + content: counter(list, lower-roman); + } + + .ck.ck-content ol.fancy-list[style*="upper-roman"] > li::before { + content: counter(list, upper-roman); + } + + .ck.ck-content ol.fancy-list[style*="lower-latin"] > li::before { + content: counter(list, lower-latin); + } + + .ck.ck-content ol.fancy-list[style*="upper-latin"] > li::before { + content: counter(list, upper-latin); + } + + .ck.ck-content ol.fancy-list > li > h4, + .ck.ck-content ul.fancy-list > li > h4 { + font-family: 'Anton', cursive; + text-transform: uppercase; + font-size: 1.8em; + padding-top: 0; + } + + .ck.ck-content ul.italic-list { + font-style: italic; + } + + .ck.ck-content li.underlined-list-item { + text-decoration: underline; + } + +

Lots of styles: block + inline

+

Top five places to visit this summer

+

...and stay on the budget.

+
    +
  1. +

    Gran Canaria, Spain

    +

    Gran Canaria is a popular tourist destination. The island is often called “a miniature continent”. This is because it offers a great variety of climates and landscapes, from golden sandy beaches through rocky ravines and white dunes to charming villages. In the mid-2010s, over 3.5 million tourists visited Gran Canaria each year.

    +
    2. +
  2. +

    Florence, Italy

    +

    Florence, established in 59 BC by Julius Caesar himself got its name from two rivers that surrounded the settlement. Thanks to centuries of history, the capital of Tuscany has plenty to offer to tourists hungry for sightseeing. Some of the most interesting places to visit include:

    + +

    Florence is also regarded as one of the top 15 fashion capitals of the world.

    +
    3. +
  3. +

    Porto, Portugal

    +

    Porto, located at the Duero river estuary in northern Portugal, is the country’s second-largest city. It is regarded as one of the oldest European centers, which was recognized in 1996 when deemed the city’s historic center a World Heritage Site.

    +
    + bar +
    Caption
    +
    +

    In 2014 and 2017, Porto was elected The Best European Destination by the Best European Destinations Agency. The city is also home to the Porto School of Architecture, one of the most prestigious architecture schools in the world.

    +
    4. +
  4. +

    Budapest, Hungary

    +

    The capital of Hungary, a spa city sitting across the Danube river, is alive with Roman, Turkish, and European history and cultural heritage. Thermal baths, the picturesque river, and the Gellért Hill citadel are only a few things worth considering while planning your trip to Hungary.

    +
    +

    They said that, of course, Budapest is beautiful. But it is in fact almost ludicrously beautiful. – Anthony Bourdain

    +
    +
    5. +
  5. +

    The Peloponnese, Greece

    +

    The Peloponnese peninsula was already inhabited in prehistoric times. During the bronze age, the first European highly developed culture – the Mycenaean civilization – started there. Roman, Byzantine, Slavic, and Ottoman rules left a significant mark on the place with numerous architectural and cultural relics worth visiting to this day.

    +
    6. +
+ +
+

Heading 1

Paragraph

Foobar
diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.js b/packages/ckeditor5-style/tests/manual/styledropdown.js index 124e205d7be..2065f91ec5f 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.js +++ b/packages/ckeditor5-style/tests/manual/styledropdown.js @@ -6,7 +6,6 @@ /* globals console, window, document */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; -import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; @@ -23,7 +22,6 @@ import HtmlEmbed from '@ckeditor/ckeditor5-html-embed/src/htmlembed'; import ImageResize from '@ckeditor/ckeditor5-image/src/imageresize'; import IndentBlock from '@ckeditor/ckeditor5-indent/src/indentblock'; import LinkImage from '@ckeditor/ckeditor5-link/src/linkimage'; -import ListProperties from '@ckeditor/ckeditor5-list/src/listproperties'; import Mention from '@ckeditor/ckeditor5-mention/src/mention'; import PageBreak from '@ckeditor/ckeditor5-page-break/src/pagebreak'; import PasteFromOffice from '@ckeditor/ckeditor5-paste-from-office/src/pastefromoffice'; @@ -37,20 +35,47 @@ import TableProperties from '@ckeditor/ckeditor5-table/src/tableproperties'; import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; import TextTransformation from '@ckeditor/ckeditor5-typing/src/texttransformation'; import TextPartLanguage from '@ckeditor/ckeditor5-language/src/textpartlanguage'; -import TodoList from '@ckeditor/ckeditor5-list/src/todolist'; import Underline from '@ckeditor/ckeditor5-basic-styles/src/underline'; import WordCount from '@ckeditor/ckeditor5-word-count/src/wordcount'; import CloudServices from '@ckeditor/ckeditor5-cloud-services/src/cloudservices'; import ImageUpload from '@ckeditor/ckeditor5-image/src/imageupload'; +import DocumentList from '@ckeditor/ckeditor5-list/src/documentlist'; +import DocumentListProperties from '@ckeditor/ckeditor5-list/src/documentlistproperties'; -import { CS_CONFIG } from '@ckeditor/ckeditor5-cloud-services/tests/_utils/cloud-services-config'; +import { Essentials } from '@ckeditor/ckeditor5-essentials'; +import { Autoformat } from '@ckeditor/ckeditor5-autoformat'; +import { BlockQuote } from '@ckeditor/ckeditor5-block-quote'; +import { Bold, Italic } from '@ckeditor/ckeditor5-basic-styles'; +import { Heading } from '@ckeditor/ckeditor5-heading'; +import { Image, ImageCaption, ImageStyle, ImageToolbar } from '@ckeditor/ckeditor5-image'; +import { Indent } from '@ckeditor/ckeditor5-indent'; +import { Link } from '@ckeditor/ckeditor5-link'; +import { MediaEmbed } from '@ckeditor/ckeditor5-media-embed'; +import { Paragraph } from '@ckeditor/ckeditor5-paragraph'; +import { Table, TableToolbar } from '@ckeditor/ckeditor5-table'; +import { CS_CONFIG } from '@ckeditor/ckeditor5-cloud-services/tests/_utils/cloud-services-config'; import Style from '../../src/style'; const config = { plugins: [ Alignment, - ArticlePluginSet, + Essentials, + Autoformat, + BlockQuote, + Bold, + Heading, + Image, + ImageCaption, + ImageStyle, + ImageToolbar, + Indent, + Italic, + Link, + MediaEmbed, + Paragraph, + Table, + TableToolbar, CloudServices, Code, CodeBlock, @@ -67,7 +92,8 @@ const config = { ImageUpload, IndentBlock, LinkImage, - ListProperties, + DocumentList, + DocumentListProperties, Mention, PageBreak, PasteFromOffice, @@ -81,7 +107,6 @@ const config = { TableProperties, TextPartLanguage, TextTransformation, - TodoList, Underline, WordCount, @@ -100,7 +125,7 @@ const config = { '|', 'highlight', 'fontSize', 'fontFamily', 'fontColor', 'fontBackgroundColor', '-', - 'bulletedList', 'numberedList', 'todoList', + 'bulletedList', 'numberedList', '|', 'blockQuote', 'uploadImage', 'insertTable', 'mediaEmbed', 'codeBlock', '|', @@ -310,6 +335,21 @@ ClassicEditor name: 'code.Qux', element: 'code', classes: [ 'Qux' ] + }, + { + name: 'Fancy list', + element: 'ol', + classes: [ 'fancy-list' ] + }, + { + name: 'Italic list', + element: 'ul', + classes: [ 'italic-list' ] + }, + { + name: 'Underlined list item', + element: 'li', + classes: [ 'underlined-list-item' ] } ] } From 1db87aaa3db91f755fed581f336ae1fbd90cd6ee Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 17 Apr 2023 11:56:44 +0200 Subject: [PATCH 02/17] Code cleaning. --- .../src/generalhtmlsupport.ts | 3 +- .../src/documentlist/documentlistutils.ts | 5 ++- packages/ckeditor5-style/src/stylecommand.ts | 42 +++++++++---------- .../tests/manual/styledropdown.html | 4 +- .../tests/manual/styledropdown.js | 4 +- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts index 2ad8787edc1..7f58e403fff 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts @@ -77,9 +77,10 @@ export default class GeneralHtmlSupport extends Plugin { /** * Returns a GHS model attribute name related to a given view element name. * + * @internal * @param viewElementName A view element name. */ - private getGhsAttributeNameForElement( viewElementName: string ): string { + public getGhsAttributeNameForElement( viewElementName: string ): string { if ( viewElementName == 'ol' || viewElementName == 'ul' ) { return 'htmlListAttributes'; } else if ( viewElementName == 'li' ) { diff --git a/packages/ckeditor5-list/src/documentlist/documentlistutils.ts b/packages/ckeditor5-list/src/documentlist/documentlistutils.ts index cc7b4493760..9f4d76a621a 100644 --- a/packages/ckeditor5-list/src/documentlist/documentlistutils.ts +++ b/packages/ckeditor5-list/src/documentlist/documentlistutils.ts @@ -57,7 +57,10 @@ export default class DocumentListUtils extends Plugin { } /** - * TODO + * Expands the given list of selected blocks to include the leading and tailing blocks of partially selected list items. + * + * @param blocks The list of selected blocks. + * @param options.withNested Whether should include nested list items. */ public expandListBlocksToCompleteItems( blocks: ArrayOrItem, options: { withNested?: boolean } = {} ): Array { return expandListBlocksToCompleteItems( blocks, options ); diff --git a/packages/ckeditor5-style/src/stylecommand.ts b/packages/ckeditor5-style/src/stylecommand.ts index c7b3b2a5590..65081eaf723 100644 --- a/packages/ckeditor5-style/src/stylecommand.ts +++ b/packages/ckeditor5-style/src/stylecommand.ts @@ -68,6 +68,7 @@ export default class StyleCommand extends Command { public override refresh(): void { const model = this.editor.model; const selection = model.document.selection; + const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? this.editor.plugins.get( 'DocumentListUtils' ) : null; @@ -131,15 +132,7 @@ export default class StyleCommand extends Command { enabledStyles.add( definition.name ); // Check if this block style is active. - let attributeName = 'htmlAttributes'; - - if ( - [ 'ol', 'ul', 'li' ].includes( definition.element ) && - documentListUtils && documentListUtils.isListItemBlock( block ) - ) { - attributeName = definition.element == 'li' ? 'htmlLiAttributes' : 'htmlListAttributes'; - } - + const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); const ghsAttributeValue = block.getAttribute( attributeName ); if ( hasAllClasses( ghsAttributeValue, definition.classes ) ) { @@ -199,6 +192,8 @@ export default class StyleCommand extends Command { const model = this.editor.model; const selection = model.document.selection; const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); + const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? + this.editor.plugins.get( 'DocumentListUtils' ) : null; const definition: BlockStyleDefinition | InlineStyleDefinition = [ ...this._styleDefinitions.inline, @@ -207,14 +202,19 @@ export default class StyleCommand extends Command { const shouldAddStyle = forceValue === undefined ? !this.value.includes( definition.name ) : forceValue; - const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? - this.editor.plugins.get( 'DocumentListUtils' ) : null; - model.change( () => { let selectables; if ( isBlockStyleDefinition( definition ) ) { - selectables = getAffectedBlocks( selection.getSelectedBlocks(), definition.modelElements, model.schema, documentListUtils ); + const selector = [ 'ol', 'ul', 'li' ].includes( definition.element ) ? { + type: 'attribute' as const, + names: [ htmlSupport.getGhsAttributeNameForElement( definition.element ) ] + } : { + type: 'element' as const, + names: definition.modelElements + }; + + selectables = getAffectedBlocks( selection.getSelectedBlocks(), selector, model.schema, documentListUtils ); } else { selectables = [ selection ]; } @@ -271,7 +271,10 @@ function hasAllClasses( ghsAttributeValue: unknown, classes: Array ): bo */ function getAffectedBlocks( selectedBlocks: IterableIterator, - elementNames: Array, + selector: { + type: 'element' | 'attribute'; + names: Array; + }, schema: Schema, documentListUtils: DocumentListUtils | null ): Set { @@ -285,24 +288,21 @@ function getAffectedBlocks( break; } - if ( documentListUtils && documentListUtils.isListItemBlock( block ) ) { - // TODO make some precise option for this - if ( elementNames.includes( 'htmlOl' ) || elementNames.includes( 'htmlUl' ) ) { + if ( selector.type == 'attribute' ) { + if ( documentListUtils && selector.names.includes( 'htmlListAttributes' ) ) { for ( const element of documentListUtils.expandListBlocksToCompleteList( block ) ) { blocks.add( element ); } break; - } else if ( elementNames.includes( 'htmlLi' ) ) { + } else if ( documentListUtils && selector.names.includes( 'htmlLiAttributes' ) ) { for ( const element of documentListUtils.expandListBlocksToCompleteItems( block, { withNested: false } ) ) { blocks.add( element ); } break; } - } - - if ( elementNames.includes( block.name ) ) { + } else if ( selector.names.includes( block.name ) ) { blocks.add( block ); break; diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.html b/packages/ckeditor5-style/tests/manual/styledropdown.html index 8285799402f..1d014949ef4 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.html +++ b/packages/ckeditor5-style/tests/manual/styledropdown.html @@ -170,8 +170,8 @@ font-style: italic; } - .ck.ck-content li.underlined-list-item { - text-decoration: underline; + .ck.ck-content li.background-list-item { + background-color: #FFAE7F; } diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.js b/packages/ckeditor5-style/tests/manual/styledropdown.js index 2065f91ec5f..43ad8b3e996 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.js +++ b/packages/ckeditor5-style/tests/manual/styledropdown.js @@ -347,9 +347,9 @@ ClassicEditor classes: [ 'italic-list' ] }, { - name: 'Underlined list item', + name: 'Background list item', element: 'li', - classes: [ 'underlined-list-item' ] + classes: [ 'background-list-item' ] } ] } From 3cdac2326fc26032e32e282e08e8b38ddcc5d904 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 17 Apr 2023 14:01:47 +0200 Subject: [PATCH 03/17] Updated style preview. --- .../src/ui/stylegridbuttonview.ts | 59 +++++++++++++++---- .../tests/manual/styledropdown.html | 7 ++- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts index a26eb299215..05b61b1e575 100644 --- a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts @@ -8,7 +8,7 @@ */ import type { Locale } from 'ckeditor5/src/utils'; -import { ButtonView, View } from 'ckeditor5/src/ui'; +import { ButtonView, View, type TemplateDefinition } from 'ckeditor5/src/ui'; import type { StyleDefinition } from '../styleconfig'; @@ -66,6 +66,51 @@ export default class StyleGridButtonView extends ButtonView { const { element, classes } = this.styleDefinition; const previewView = new View( this.locale ); + const children: Array = []; + const text = 'AaBbCcDdEeFfGgHhIiJj'; + + if ( element == 'ol' || element == 'ul' ) { + children.push( { + tag: element, + attributes: { + class: classes + }, + children: [ + { + tag: 'li', + children: [ + { text } + ] + } + ] + } ); + } else if ( element == 'li' ) { + children.push( { + tag: 'ol', + children: [ + { + tag: element, + attributes: { + class: classes + }, + children: [ + { text } + ] + } + ] + } ); + } else { + children.push( { + tag: this._isPreviewable( element ) ? element : 'div', + attributes: { + class: classes + }, + children: [ + { text } + ] + } ); + } + previewView.setTemplate( { tag: 'div', @@ -80,17 +125,7 @@ export default class StyleGridButtonView extends ButtonView { 'aria-hidden': 'true' }, - children: [ - { - tag: this._isPreviewable( element ) ? element : 'div', - attributes: { - class: classes - }, - children: [ - { text: 'AaBbCcDdEeFfGgHhIiJj' } - ] - } - ] + children } ); return previewView; diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.html b/packages/ckeditor5-style/tests/manual/styledropdown.html index 1d014949ef4..5c9ebdd8055 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.html +++ b/packages/ckeditor5-style/tests/manual/styledropdown.html @@ -109,11 +109,14 @@ color: hsl(0, 0%, 40%); } + .ck.ck-content ol.fancy-list { + padding-left: 30px; + } .ck.ck-content ol.fancy-list > li { counter-increment: list; list-style: none; margin-bottom: 2.5em; - padding-left: 70px; + padding-left: 50px; } .ck.ck-content ol.fancy-list > li::before { @@ -123,7 +126,7 @@ width: 50px; height: 50px; float: left; - margin: 5px 20px 0 -78px; + margin: 5px 10px 0 -68px; color: #fff; background: #000; clip-path: polygon(0 0, 100% 0, 100% 65%, 65% 100%, 0 100%); From 840cb3ee39f6d68e2d1201befcc7fa0d76c2b32a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 17 Apr 2023 18:00:58 +0200 Subject: [PATCH 04/17] Extracted custom list handling to an integration class. --- .../src/integrations/documentlist.ts | 130 +++++++++++++++ packages/ckeditor5-style/src/style.ts | 3 +- packages/ckeditor5-style/src/stylecommand.ts | 150 +++++------------- packages/ckeditor5-style/src/styleutils.ts | 79 ++++++++- 4 files changed, 253 insertions(+), 109 deletions(-) create mode 100644 packages/ckeditor5-style/src/integrations/documentlist.ts diff --git a/packages/ckeditor5-style/src/integrations/documentlist.ts b/packages/ckeditor5-style/src/integrations/documentlist.ts new file mode 100644 index 00000000000..2e6a288bbea --- /dev/null +++ b/packages/ckeditor5-style/src/integrations/documentlist.ts @@ -0,0 +1,130 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import { Plugin } from 'ckeditor5/src/core'; +import type { Element } from 'ckeditor5/src/engine'; +import type { DocumentListUtils } from '@ckeditor/ckeditor5-list'; + +import StyleUtils, { + type BlockStyleDefinition, + type StyleUtilsGetAffectedBlocksEvent, + type StyleUtilsIsActiveForBlockEvent, + type StyleUtilsIsEnabledForBlockEvent +} from '../styleutils'; +import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; + +/** + * @module style/integrations/documentliststylesupport + */ + +export default class DocumentListStyleSupport extends Plugin { + private _documentListUtils!: DocumentListUtils; + private _styleUtils!: StyleUtils; + + /** + * @inheritDoc + */ + public static get pluginName(): 'DocumentListStyleSupport' { + return 'DocumentListStyleSupport'; + } + + /** + * @inheritDoc + */ + public static get requires() { + return [ StyleUtils ] as const; + } + + /** + * @inheritDoc + */ + public init(): void { + const editor = this.editor; + + if ( !editor.plugins.has( 'DocumentList' ) ) { + return; + } + + this._styleUtils = editor.plugins.get( StyleUtils ); + this._documentListUtils = this.editor.plugins.get( 'DocumentListUtils' ); + + this.listenTo( this._styleUtils, 'isStyleEnabledForBlock', ( evt, [ definition, block ] ) => { + if ( this._isStyleEnabledForBlock( definition, block ) ) { + evt.return = true; + evt.stop(); + } + }, { priority: 'high' } ); + + this.listenTo( this._styleUtils, 'isStyleActiveForBlock', ( evt, [ definition, block ] ) => { + if ( this._isStyleActiveForBlock( definition, block ) ) { + evt.return = true; + evt.stop(); + } + }, { priority: 'high' } ); + + this.listenTo( this._styleUtils, 'getAffectedBlocks', ( evt, [ definition, block ] ) => { + const blocks = this._getAffectedBlocks( definition, block ); + + if ( blocks ) { + evt.return = blocks; + evt.stop(); + } + }, { priority: 'high' } ); + } + + /** + * TODO + */ + private _isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { + const model = this.editor.model; + + if ( ![ 'ol', 'ul', 'li' ].includes( definition.element ) ) { + return false; + } + + if ( !this._documentListUtils.isListItemBlock( block ) ) { + return false; + } + + if ( definition.element == 'ol' || definition.element == 'ul' ) { + if ( !model.schema.checkAttribute( block, 'htmlListAttributes' ) ) { + return false; + } + + const viewElementName = block.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; + + return definition.element == viewElementName; + } else { + return model.schema.checkAttribute( block, 'htmlLiAttributes' ); + } + } + + /** + * TODO + */ + private _isStyleActiveForBlock( definition: BlockStyleDefinition, block: Element ): boolean { + const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); + + const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); + const ghsAttributeValue = block.getAttribute( attributeName ); + + return this._styleUtils.hasAllClasses( ghsAttributeValue, definition.classes ); + } + + /** + * TODO + */ + private _getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Iterable | null { + if ( !this._isStyleEnabledForBlock( definition, block ) ) { + return null; + } + + if ( definition.element == 'li' ) { + return this._documentListUtils.expandListBlocksToCompleteItems( block, { withNested: false } ); + } else { + return this._documentListUtils.expandListBlocksToCompleteList( block ); + } + } +} diff --git a/packages/ckeditor5-style/src/style.ts b/packages/ckeditor5-style/src/style.ts index a0f8d99d2a3..d8421f43ec8 100644 --- a/packages/ckeditor5-style/src/style.ts +++ b/packages/ckeditor5-style/src/style.ts @@ -11,6 +11,7 @@ import { Plugin } from 'ckeditor5/src/core'; import StyleUI from './styleui'; import StyleEditing from './styleediting'; +import DocumentListStyleSupport from './integrations/documentlist'; /** * The style plugin. @@ -30,6 +31,6 @@ export default class Style extends Plugin { * @inheritDoc */ public static get requires() { - return [ StyleEditing, StyleUI ] as const; + return [ StyleEditing, StyleUI, DocumentListStyleSupport ] as const; } } diff --git a/packages/ckeditor5-style/src/stylecommand.ts b/packages/ckeditor5-style/src/stylecommand.ts index 65081eaf723..3395714186f 100644 --- a/packages/ckeditor5-style/src/stylecommand.ts +++ b/packages/ckeditor5-style/src/stylecommand.ts @@ -11,10 +11,12 @@ import type { Element, Schema } from 'ckeditor5/src/engine'; import { Command, type Editor } from 'ckeditor5/src/core'; import { logWarning, first } from 'ckeditor5/src/utils'; import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; -import type { DocumentListUtils } from '@ckeditor/ckeditor5-list'; -import { isObject } from 'lodash-es'; -import type { BlockStyleDefinition, InlineStyleDefinition, NormalizedStyleDefinitions } from './styleutils'; +import StyleUtils, { + type BlockStyleDefinition, + type InlineStyleDefinition, + type NormalizedStyleDefinitions +} from './styleutils'; /** * Style command. @@ -68,9 +70,7 @@ export default class StyleCommand extends Command { public override refresh(): void { const model = this.editor.model; const selection = model.document.selection; - const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); - const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? - this.editor.plugins.get( 'DocumentListUtils' ) : null; + const styleUtils: StyleUtils = this.editor.plugins.get( StyleUtils ); const value = new Set(); const enabledStyles = new Set(); @@ -86,7 +86,7 @@ export default class StyleCommand extends Command { // Check if this inline style is active. const ghsAttributeValue = this._getValueFromFirstAllowedNode( ghsAttributeName ); - if ( hasAllClasses( ghsAttributeValue, definition.classes ) ) { + if ( styleUtils.hasAllClasses( ghsAttributeValue, definition.classes ) ) { value.add( definition.name ); } } @@ -105,37 +105,16 @@ export default class StyleCommand extends Command { break; } - if ( - !model.schema.checkAttribute( block, 'htmlAttributes' ) && - !model.schema.checkAttribute( block, 'htmlListAttributes' ) && - !model.schema.checkAttribute( block, 'htmlLiAttributes' ) - ) { - continue; - } - for ( const definition of this._styleDefinitions.block ) { // Check if this block style is enabled. - if ( documentListUtils && documentListUtils.isListItemBlock( block ) ) { - if ( definition.element == 'ol' || definition.element == 'ul' ) { - const viewElementName = block.getAttribute( 'listType' ) == 'numbered' ? 'ol' : 'ul'; - - if ( definition.element != viewElementName ) { - continue; - } - } else if ( definition.element != 'li' && !definition.modelElements.includes( block.name ) ) { - continue; - } - } else if ( !definition.modelElements.includes( block.name ) ) { + if ( !styleUtils.isStyleEnabledForBlock( definition, block ) ) { continue; } enabledStyles.add( definition.name ); // Check if this block style is active. - const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); - const ghsAttributeValue = block.getAttribute( attributeName ); - - if ( hasAllClasses( ghsAttributeValue, definition.classes ) ) { + if ( styleUtils.isStyleActiveForBlock( definition, block ) ) { value.add( definition.name ); } } @@ -192,8 +171,6 @@ export default class StyleCommand extends Command { const model = this.editor.model; const selection = model.document.selection; const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); - const documentListUtils: DocumentListUtils | null = this.editor.plugins.has( 'DocumentListUtils' ) ? - this.editor.plugins.get( 'DocumentListUtils' ) : null; const definition: BlockStyleDefinition | InlineStyleDefinition = [ ...this._styleDefinitions.inline, @@ -206,15 +183,7 @@ export default class StyleCommand extends Command { let selectables; if ( isBlockStyleDefinition( definition ) ) { - const selector = [ 'ol', 'ul', 'li' ].includes( definition.element ) ? { - type: 'attribute' as const, - names: [ htmlSupport.getGhsAttributeNameForElement( definition.element ) ] - } : { - type: 'element' as const, - names: definition.modelElements - }; - - selectables = getAffectedBlocks( selection.getSelectedBlocks(), selector, model.schema, documentListUtils ); + selectables = this._findAffectedBlocks( selection.getSelectedBlocks(), definition, model.schema ); } else { selectables = [ selection ]; } @@ -229,6 +198,40 @@ export default class StyleCommand extends Command { } ); } + /** + * Returns a set of elements that should be affected by the block-style change. + */ + private _findAffectedBlocks( + selectedBlocks: IterableIterator, + definition: BlockStyleDefinition, + schema: Schema + ): Set { + const styleUtils: StyleUtils = this.editor.plugins.get( StyleUtils ); + const blocks = new Set(); + + for ( const selectedBlock of selectedBlocks ) { + const ancestorBlocks: Array = selectedBlock.getAncestors( { includeSelf: true, parentFirst: true } ) as Array; + + for ( const block of ancestorBlocks ) { + if ( schema.isLimit( block ) ) { + break; + } + + const affectedBlocks = styleUtils.getAffectedBlocks( definition, block ); + + if ( affectedBlocks ) { + for ( const affectedBlock of affectedBlocks ) { + blocks.add( affectedBlock ); + } + + break; + } + } + } + + return blocks; + } + /** * Checks the attribute value of the first node in the selection that allows the attribute. * For the collapsed selection, returns the selection attribute. @@ -257,74 +260,9 @@ export default class StyleCommand extends Command { } } -/** - * Verifies if all classes are present in the given GHS attribute. - */ -function hasAllClasses( ghsAttributeValue: unknown, classes: Array ): boolean { - return isObject( ghsAttributeValue ) && - hasClassesProperty( ghsAttributeValue ) && - classes.every( className => ghsAttributeValue.classes.includes( className ) ); -} - -/** - * Returns a set of elements that should be affected by the block-style change. - */ -function getAffectedBlocks( - selectedBlocks: IterableIterator, - selector: { - type: 'element' | 'attribute'; - names: Array; - }, - schema: Schema, - documentListUtils: DocumentListUtils | null -): Set { - const blocks = new Set(); - - for ( const selectedBlock of selectedBlocks ) { - const ancestorBlocks: Array = selectedBlock.getAncestors( { includeSelf: true, parentFirst: true } ) as Array; - - for ( const block of ancestorBlocks ) { - if ( schema.isLimit( block ) ) { - break; - } - - if ( selector.type == 'attribute' ) { - if ( documentListUtils && selector.names.includes( 'htmlListAttributes' ) ) { - for ( const element of documentListUtils.expandListBlocksToCompleteList( block ) ) { - blocks.add( element ); - } - - break; - } else if ( documentListUtils && selector.names.includes( 'htmlLiAttributes' ) ) { - for ( const element of documentListUtils.expandListBlocksToCompleteItems( block, { withNested: false } ) ) { - blocks.add( element ); - } - - break; - } - } else if ( selector.names.includes( block.name ) ) { - blocks.add( block ); - - break; - } - } - } - - return blocks; -} - /** * Checks if provided style definition is of type block. */ function isBlockStyleDefinition( definition: BlockStyleDefinition | InlineStyleDefinition ): definition is BlockStyleDefinition { return 'isBlock' in definition; } - -/** - * Checks if given object has `classes` property which is an array. - * - * @param obj Object to check. - */ -function hasClassesProperty }>( obj: T ): obj is T & { classes: Array } { - return Boolean( obj.classes ) && Array.isArray( obj.classes ); -} diff --git a/packages/ckeditor5-style/src/styleutils.ts b/packages/ckeditor5-style/src/styleutils.ts index 051d00b4e66..f41c7cbbb77 100644 --- a/packages/ckeditor5-style/src/styleutils.ts +++ b/packages/ckeditor5-style/src/styleutils.ts @@ -7,10 +7,13 @@ * @module style/styleutils */ -import { Plugin } from 'ckeditor5/src/core'; -import type { DataSchema } from '@ckeditor/ckeditor5-html-support'; +import { Plugin, type Editor } from 'ckeditor5/src/core'; +import type { Element } from 'ckeditor5/src/engine'; +import type { DecoratedMethodEvent } from 'ckeditor5/src/utils'; +import type { DataSchema, GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; import type { StyleDefinition } from './styleconfig'; +import { isObject } from 'lodash-es'; export default class StyleUtils extends Plugin { /** @@ -20,6 +23,17 @@ export default class StyleUtils extends Plugin { return 'StyleUtils'; } + /** + * @inheritDoc + */ + constructor( editor: Editor ) { + super( editor ); + + this.decorate( 'isStyleEnabledForBlock' ); + this.decorate( 'isStyleActiveForBlock' ); + this.decorate( 'getAffectedBlocks' ); + } + /** * Normalizes {@link module:style/styleconfig~StyleConfig#definitions} in the configuration of the styles feature. * The structure of normalized styles looks as follows: @@ -70,6 +84,63 @@ export default class StyleUtils extends Plugin { } return normalizedDefinitions; } + + /** + * TODO + * @internal + */ + public isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { + const model = this.editor.model; + + if ( !model.schema.checkAttribute( block, 'htmlAttributes' ) ) { + return false; + } + + return definition.modelElements.includes( block.name ); + } + + /** + * TODO + * @internal + */ + public isStyleActiveForBlock( definition: BlockStyleDefinition, block: Element ): boolean { + const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); + const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); + const ghsAttributeValue = block.getAttribute( attributeName ); + + return this.hasAllClasses( ghsAttributeValue, definition.classes ); + } + + /** + * TODO + */ + public getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Iterable | null { + if ( definition.modelElements.includes( block.name ) ) { + return [ block ]; + } + + return null; + } + + /** + * Verifies if all classes are present in the given GHS attribute. + * + * @internal + */ + public hasAllClasses( ghsAttributeValue: unknown, classes: Array ): boolean { + return isObject( ghsAttributeValue ) && + hasClassesProperty( ghsAttributeValue ) && + classes.every( className => ghsAttributeValue.classes.includes( className ) ); + } +} + +/** + * Checks if given object has `classes` property which is an array. + * + * @param obj Object to check. + */ +function hasClassesProperty }>( obj: T ): obj is T & { classes: Array } { + return Boolean( obj.classes ) && Array.isArray( obj.classes ); } export interface NormalizedStyleDefinitions { @@ -85,3 +156,7 @@ export interface BlockStyleDefinition extends StyleDefinition { export interface InlineStyleDefinition extends StyleDefinition { ghsAttributes: Array; } + +export type StyleUtilsIsEnabledForBlockEvent = DecoratedMethodEvent; +export type StyleUtilsIsActiveForBlockEvent = DecoratedMethodEvent; +export type StyleUtilsGetAffectedBlocksEvent = DecoratedMethodEvent; From 073361fc190a1fad35fcf8a846c569f17eb7e157 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 17 Apr 2023 18:40:43 +0200 Subject: [PATCH 05/17] Added support for setting style for widgets. --- .../src/integrations/mediaembed.ts | 3 ++- .../src/integrations/table.ts | 9 +++++++-- packages/ckeditor5-style/src/stylecommand.ts | 19 +++++++++++-------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-html-support/src/integrations/mediaembed.ts b/packages/ckeditor5-html-support/src/integrations/mediaembed.ts index fb1798b1c5c..7d3526bb3ab 100644 --- a/packages/ckeditor5-html-support/src/integrations/mediaembed.ts +++ b/packages/ckeditor5-html-support/src/integrations/mediaembed.ts @@ -150,7 +150,8 @@ function modelToViewMediaAttributeConverter( mediaElementName: string ) { conversionApi.writer, attributeOldValue as GHSViewAttributes, attributeNewValue as GHSViewAttributes, - viewElement! ); + viewElement! + ); } ); } }; diff --git a/packages/ckeditor5-html-support/src/integrations/table.ts b/packages/ckeditor5-html-support/src/integrations/table.ts index 9ed7ae1ea16..a3b67c965eb 100644 --- a/packages/ckeditor5-html-support/src/integrations/table.ts +++ b/packages/ckeditor5-html-support/src/integrations/table.ts @@ -15,7 +15,7 @@ import type { UpcastElementEvent, ViewElement } from 'ckeditor5/src/engine'; import { Plugin } from 'ckeditor5/src/core'; -import { setViewAttributes, type GHSViewAttributes } from '../conversionutils'; +import { updateViewAttributes, type GHSViewAttributes } from '../conversionutils'; import DataFilter, { type DataFilterRegisterEvent } from '../datafilter'; import { getDescendantElement } from './integrationutils'; @@ -157,7 +157,12 @@ function modelToViewTableAttributeConverter() { const containerElement = conversionApi.mapper.toViewElement( data.item as Element ); const viewElement = getDescendantElement( conversionApi.writer, containerElement!, elementName ); - setViewAttributes( conversionApi.writer, data.attributeNewValue as GHSViewAttributes, viewElement! ); + updateViewAttributes( + conversionApi.writer, + data.attributeOldValue as GHSViewAttributes, + data.attributeNewValue as GHSViewAttributes, + viewElement! + ); } ); } }; diff --git a/packages/ckeditor5-style/src/stylecommand.ts b/packages/ckeditor5-style/src/stylecommand.ts index 3395714186f..7eae8ee944f 100644 --- a/packages/ckeditor5-style/src/stylecommand.ts +++ b/packages/ckeditor5-style/src/stylecommand.ts @@ -99,9 +99,7 @@ export default class StyleCommand extends Command { const ancestorBlocks = firstBlock.getAncestors( { includeSelf: true, parentFirst: true } ) as Array; for ( const block of ancestorBlocks ) { - // E.g. reached a model table when the selection is in a cell. The command should not modify - // ancestors of a table. - if ( model.schema.isLimit( block ) ) { + if ( block.is( 'rootElement' ) ) { break; } @@ -118,6 +116,12 @@ export default class StyleCommand extends Command { value.add( definition.name ); } } + + // E.g. reached a model table when the selection is in a cell. The command should not modify + // ancestors of a table. + if ( model.schema.isObject( block ) ) { + break; + } } } @@ -183,7 +187,7 @@ export default class StyleCommand extends Command { let selectables; if ( isBlockStyleDefinition( definition ) ) { - selectables = this._findAffectedBlocks( selection.getSelectedBlocks(), definition, model.schema ); + selectables = this._findAffectedBlocks( selection.getSelectedBlocks(), definition ); } else { selectables = [ selection ]; } @@ -203,17 +207,16 @@ export default class StyleCommand extends Command { */ private _findAffectedBlocks( selectedBlocks: IterableIterator, - definition: BlockStyleDefinition, - schema: Schema + definition: BlockStyleDefinition ): Set { const styleUtils: StyleUtils = this.editor.plugins.get( StyleUtils ); const blocks = new Set(); for ( const selectedBlock of selectedBlocks ) { - const ancestorBlocks: Array = selectedBlock.getAncestors( { includeSelf: true, parentFirst: true } ) as Array; + const ancestorBlocks = selectedBlock.getAncestors( { includeSelf: true, parentFirst: true } ) as Array; for ( const block of ancestorBlocks ) { - if ( schema.isLimit( block ) ) { + if ( block.is( 'rootElement' ) ) { break; } From 3c607b5b6f29c51aa2da66cf6b8836810f71dae3 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 17 Apr 2023 19:15:02 +0200 Subject: [PATCH 06/17] Code cleaning. --- packages/ckeditor5-html-support/src/dataschema.ts | 8 +------- packages/ckeditor5-html-support/src/schemadefinitions.ts | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/ckeditor5-html-support/src/dataschema.ts b/packages/ckeditor5-html-support/src/dataschema.ts index 5bb550cfa57..71ef4a323ea 100644 --- a/packages/ckeditor5-html-support/src/dataschema.ts +++ b/packages/ckeditor5-html-support/src/dataschema.ts @@ -49,13 +49,7 @@ export default class DataSchema extends Plugin { /** * A map of registered data schema definitions. */ - private readonly _definitions: Map; - - constructor( editor: Editor ) { - super( editor ); - - this._definitions = new Map(); - } + private readonly _definitions = new Map(); /** * @inheritDoc diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.ts b/packages/ckeditor5-html-support/src/schemadefinitions.ts index 832accf9024..ebd58c5d540 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.ts +++ b/packages/ckeditor5-html-support/src/schemadefinitions.ts @@ -86,11 +86,11 @@ export default { }, { model: 'tableCell', - view: 'td' + view: 'th' }, { model: 'tableCell', - view: 'th' + view: 'td' }, { model: 'tableColumnGroup', From 96177e4b8f11fe1a6890f755eab41f4c27c8faff Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 18 Apr 2023 12:54:35 +0200 Subject: [PATCH 07/17] Added extendable preview for style. --- .../src/integrations/documentlist.ts | 66 ++++++++++++++++-- packages/ckeditor5-style/src/style.ts | 3 +- packages/ckeditor5-style/src/styleconfig.ts | 3 + packages/ckeditor5-style/src/styleediting.ts | 3 +- packages/ckeditor5-style/src/styleutils.ts | 51 +++++++++++++- .../src/ui/stylegridbuttonview.ts | 69 ++----------------- .../ckeditor5-style/tests/stylecommand.js | 5 +- .../ckeditor5-style/tests/styleediting.js | 5 +- 8 files changed, 125 insertions(+), 80 deletions(-) diff --git a/packages/ckeditor5-style/src/integrations/documentlist.ts b/packages/ckeditor5-style/src/integrations/documentlist.ts index 2e6a288bbea..5022e3519fe 100644 --- a/packages/ckeditor5-style/src/integrations/documentlist.ts +++ b/packages/ckeditor5-style/src/integrations/documentlist.ts @@ -6,14 +6,19 @@ import { Plugin } from 'ckeditor5/src/core'; import type { Element } from 'ckeditor5/src/engine'; import type { DocumentListUtils } from '@ckeditor/ckeditor5-list'; +import type { TemplateDefinition } from 'ckeditor5/src/ui'; + +import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; import StyleUtils, { type BlockStyleDefinition, type StyleUtilsGetAffectedBlocksEvent, type StyleUtilsIsActiveForBlockEvent, - type StyleUtilsIsEnabledForBlockEvent + type StyleUtilsIsEnabledForBlockEvent, + type StyleUtilsGetStylePreviewEvent } from '../styleutils'; -import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; + +import type { StyleDefinition } from '../styleconfig'; /** * @module style/integrations/documentliststylesupport @@ -43,7 +48,7 @@ export default class DocumentListStyleSupport extends Plugin { public init(): void { const editor = this.editor; - if ( !editor.plugins.has( 'DocumentList' ) ) { + if ( !editor.plugins.has( 'DocumentListEditing' ) ) { return; } @@ -72,6 +77,15 @@ export default class DocumentListStyleSupport extends Plugin { evt.stop(); } }, { priority: 'high' } ); + + this.listenTo( this._styleUtils, 'getStylePreview', ( evt, [ definition, children ] ) => { + const templateDefinition = this._getStylePreview( definition, children ); + + if ( templateDefinition ) { + evt.return = templateDefinition; + evt.stop(); + } + }, { priority: 'high' } ); } /** @@ -79,6 +93,7 @@ export default class DocumentListStyleSupport extends Plugin { */ private _isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { const model = this.editor.model; + const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); if ( ![ 'ol', 'ul', 'li' ].includes( definition.element ) ) { return false; @@ -88,8 +103,10 @@ export default class DocumentListStyleSupport extends Plugin { return false; } + const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); + if ( definition.element == 'ol' || definition.element == 'ul' ) { - if ( !model.schema.checkAttribute( block, 'htmlListAttributes' ) ) { + if ( !model.schema.checkAttribute( block, attributeName ) ) { return false; } @@ -97,7 +114,7 @@ export default class DocumentListStyleSupport extends Plugin { return definition.element == viewElementName; } else { - return model.schema.checkAttribute( block, 'htmlLiAttributes' ); + return model.schema.checkAttribute( block, attributeName ); } } @@ -116,7 +133,7 @@ export default class DocumentListStyleSupport extends Plugin { /** * TODO */ - private _getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Iterable | null { + private _getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Array | null { if ( !this._isStyleEnabledForBlock( definition, block ) ) { return null; } @@ -127,4 +144,41 @@ export default class DocumentListStyleSupport extends Plugin { return this._documentListUtils.expandListBlocksToCompleteList( block ); } } + + /** + * TODO + */ + private _getStylePreview( definition: StyleDefinition, children: Iterable ): TemplateDefinition | null { + const { element, classes } = definition; + + if ( element == 'ol' || element == 'ul' ) { + return { + tag: element, + attributes: { + class: classes + }, + children: [ + { + tag: 'li', + children + } + ] + }; + } else if ( element == 'li' ) { + return { + tag: 'ol', + children: [ + { + tag: element, + attributes: { + class: classes + }, + children + } + ] + }; + } + + return null; + } } diff --git a/packages/ckeditor5-style/src/style.ts b/packages/ckeditor5-style/src/style.ts index d8421f43ec8..a0f8d99d2a3 100644 --- a/packages/ckeditor5-style/src/style.ts +++ b/packages/ckeditor5-style/src/style.ts @@ -11,7 +11,6 @@ import { Plugin } from 'ckeditor5/src/core'; import StyleUI from './styleui'; import StyleEditing from './styleediting'; -import DocumentListStyleSupport from './integrations/documentlist'; /** * The style plugin. @@ -31,6 +30,6 @@ export default class Style extends Plugin { * @inheritDoc */ public static get requires() { - return [ StyleEditing, StyleUI, DocumentListStyleSupport ] as const; + return [ StyleEditing, StyleUI ] as const; } } diff --git a/packages/ckeditor5-style/src/styleconfig.ts b/packages/ckeditor5-style/src/styleconfig.ts index 2f552553f38..850631b15fb 100644 --- a/packages/ckeditor5-style/src/styleconfig.ts +++ b/packages/ckeditor5-style/src/styleconfig.ts @@ -7,6 +7,8 @@ * @module style/styleconfig */ +import type { TemplateDefinition } from 'ckeditor5/src/ui'; + /** * The configuration of the style feature. * @@ -88,4 +90,5 @@ export interface StyleDefinition { name: string; element: string; classes: Array; + previewTemplate?: TemplateDefinition; } diff --git a/packages/ckeditor5-style/src/styleediting.ts b/packages/ckeditor5-style/src/styleediting.ts index 771c42a2516..7843423cd93 100644 --- a/packages/ckeditor5-style/src/styleediting.ts +++ b/packages/ckeditor5-style/src/styleediting.ts @@ -14,6 +14,7 @@ import type { DataFilter, DataSchema } from '@ckeditor/ckeditor5-html-support'; import StyleCommand from './stylecommand'; import StyleUtils, { type NormalizedStyleDefinitions } from './styleutils'; import type { StyleConfig, StyleDefinition } from './styleconfig'; +import DocumentListStyleSupport from './integrations/documentlist'; /** * The style engine feature. @@ -34,7 +35,7 @@ export default class StyleEditing extends Plugin { * @inheritDoc */ public static get requires() { - return [ 'GeneralHtmlSupport', StyleUtils ] as const; + return [ 'GeneralHtmlSupport', StyleUtils, DocumentListStyleSupport ] as const; } /** diff --git a/packages/ckeditor5-style/src/styleutils.ts b/packages/ckeditor5-style/src/styleutils.ts index f41c7cbbb77..b1f3b40e40d 100644 --- a/packages/ckeditor5-style/src/styleutils.ts +++ b/packages/ckeditor5-style/src/styleutils.ts @@ -10,11 +10,19 @@ import { Plugin, type Editor } from 'ckeditor5/src/core'; import type { Element } from 'ckeditor5/src/engine'; import type { DecoratedMethodEvent } from 'ckeditor5/src/utils'; +import type { TemplateDefinition } from 'ckeditor5/src/ui'; + import type { DataSchema, GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; import type { StyleDefinition } from './styleconfig'; import { isObject } from 'lodash-es'; +// These are intermediate element names that can't be rendered as style preview because they don't make sense standalone. +const NON_PREVIEWABLE_ELEMENT_NAMES = [ + 'caption', 'colgroup', 'dd', 'dt', 'figcaption', 'legend', 'li', 'optgroup', 'option', 'rp', + 'rt', 'summary', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr' +]; + export default class StyleUtils extends Plugin { /** * @inheritDoc @@ -32,6 +40,7 @@ export default class StyleUtils extends Plugin { this.decorate( 'isStyleEnabledForBlock' ); this.decorate( 'isStyleActiveForBlock' ); this.decorate( 'getAffectedBlocks' ); + this.decorate( 'getStylePreview' ); } /** @@ -76,12 +85,19 @@ export default class StyleUtils extends Plugin { } } + if ( !definition.previewTemplate ) { + definition.previewTemplate = this.getStylePreview( definition, [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] ); + } + if ( modelElements.length ) { normalizedDefinitions.block.push( { ...definition, modelElements, isBlock: true } ); } else { normalizedDefinitions.inline.push( { ...definition, ghsAttributes } ); } } + return normalizedDefinitions; } @@ -91,8 +107,10 @@ export default class StyleUtils extends Plugin { */ public isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { const model = this.editor.model; + const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); + const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); - if ( !model.schema.checkAttribute( block, 'htmlAttributes' ) ) { + if ( !model.schema.checkAttribute( block, attributeName ) ) { return false; } @@ -114,7 +132,7 @@ export default class StyleUtils extends Plugin { /** * TODO */ - public getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Iterable | null { + public getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Array | null { if ( definition.modelElements.includes( block.name ) ) { return [ block ]; } @@ -122,6 +140,23 @@ export default class StyleUtils extends Plugin { return null; } + /** + * Returns the `TemplateDefinition` used by styles dropdown to render style preview. + * + * @internal + */ + public getStylePreview( definition: StyleDefinition, children: Iterable ): TemplateDefinition { + const { element, classes } = definition; + + return { + tag: isPreviewable( element ) ? element : 'div', + attributes: { + class: classes + }, + children + }; + } + /** * Verifies if all classes are present in the given GHS attribute. * @@ -143,6 +178,17 @@ function hasClassesProperty }>( obj: T ): o return Boolean( obj.classes ) && Array.isArray( obj.classes ); } +/** + * Decides whether an element should be created in the preview or a substitute `
` should + * be used instead. This avoids previewing a standalone ``, `
  • `, etc. without a parent. + * + * @param elementName Name of the element + * @returns Boolean indicating whether the element can be rendered. + */ +function isPreviewable( elementName: string ): boolean { + return !NON_PREVIEWABLE_ELEMENT_NAMES.includes( elementName ); +} + export interface NormalizedStyleDefinitions { block: Array; inline: Array; @@ -160,3 +206,4 @@ export interface InlineStyleDefinition extends StyleDefinition { export type StyleUtilsIsEnabledForBlockEvent = DecoratedMethodEvent; export type StyleUtilsIsActiveForBlockEvent = DecoratedMethodEvent; export type StyleUtilsGetAffectedBlocksEvent = DecoratedMethodEvent; +export type StyleUtilsGetStylePreviewEvent = DecoratedMethodEvent; diff --git a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts index 05b61b1e575..bd7b2490166 100644 --- a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts @@ -8,16 +8,10 @@ */ import type { Locale } from 'ckeditor5/src/utils'; -import { ButtonView, View, type TemplateDefinition } from 'ckeditor5/src/ui'; +import { ButtonView, View } from 'ckeditor5/src/ui'; import type { StyleDefinition } from '../styleconfig'; -// These are intermediate element names that can't be rendered as style preview because they don't make sense standalone. -const NON_PREVIEWABLE_ELEMENT_NAMES = [ - 'caption', 'colgroup', 'dd', 'dt', 'figcaption', 'legend', 'li', 'optgroup', 'option', 'rp', - 'rt', 'summary', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr' -]; - /** * A class representing an individual button (style) in the grid. Renders a rich preview of the style. */ @@ -63,54 +57,8 @@ export default class StyleGridButtonView extends ButtonView { * Creates the view representing the preview of the style. */ private _createPreview(): View { - const { element, classes } = this.styleDefinition; const previewView = new View( this.locale ); - const children: Array = []; - const text = 'AaBbCcDdEeFfGgHhIiJj'; - - if ( element == 'ol' || element == 'ul' ) { - children.push( { - tag: element, - attributes: { - class: classes - }, - children: [ - { - tag: 'li', - children: [ - { text } - ] - } - ] - } ); - } else if ( element == 'li' ) { - children.push( { - tag: 'ol', - children: [ - { - tag: element, - attributes: { - class: classes - }, - children: [ - { text } - ] - } - ] - } ); - } else { - children.push( { - tag: this._isPreviewable( element ) ? element : 'div', - attributes: { - class: classes - }, - children: [ - { text } - ] - } ); - } - previewView.setTemplate( { tag: 'div', @@ -125,20 +73,11 @@ export default class StyleGridButtonView extends ButtonView { 'aria-hidden': 'true' }, - children + children: [ + this.styleDefinition.previewTemplate! + ] } ); return previewView; } - - /** - * Decides whether an element should be created in the preview or a substitute `
    ` should - * be used instead. This avoids previewing a standalone ``, `
  • `, etc. without a parent. - * - * @param elementName Name of the element - * @returns Boolean indicating whether the element can be rendered. - */ - private _isPreviewable( elementName: string ): boolean { - return !NON_PREVIEWABLE_ELEMENT_NAMES.includes( elementName ); - } } diff --git a/packages/ckeditor5-style/tests/stylecommand.js b/packages/ckeditor5-style/tests/stylecommand.js index 50ac61fdc27..96a7042b551 100644 --- a/packages/ckeditor5-style/tests/stylecommand.js +++ b/packages/ckeditor5-style/tests/stylecommand.js @@ -218,7 +218,7 @@ describe( 'StyleCommand', () => { ] ); } ); - it( 'should not enable styles for elements outside a limit element', () => { + it( 'should not enable styles for elements outside an object element', () => { setData( model, '
    ' + '' + @@ -235,7 +235,8 @@ describe( 'StyleCommand', () => { expect( command.enabledStyles ).to.have.members( [ ...inlineStyles.map( ( { name } ) => name ), - ...blockParagraphStyles.map( ( { name } ) => name ) + ...blockParagraphStyles.map( ( { name } ) => name ), + ...blockWidgetStyles.map( ( { name } ) => name ) ] ); } ); diff --git a/packages/ckeditor5-style/tests/styleediting.js b/packages/ckeditor5-style/tests/styleediting.js index 78de2771ad6..9aec4403194 100644 --- a/packages/ckeditor5-style/tests/styleediting.js +++ b/packages/ckeditor5-style/tests/styleediting.js @@ -11,6 +11,7 @@ import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtml import StyleEditing from '../src/styleediting'; import StyleCommand from '../src/stylecommand'; import StyleUtils from '../src/styleutils'; +import DocumentListStyleSupport from '../src/integrations/documentlist'; describe( 'StyleEditing', () => { let editor, editorElement; @@ -68,8 +69,8 @@ describe( 'StyleEditing', () => { expect( StyleEditing.pluginName ).to.equal( 'StyleEditing' ); } ); - it( 'should soft-require the GHS plugin and require utils', () => { - expect( StyleEditing.requires ).to.deep.equal( [ 'GeneralHtmlSupport', StyleUtils ] ); + it( 'should soft-require the GHS plugin, and require utils, and integrations', () => { + expect( StyleEditing.requires ).to.deep.equal( [ 'GeneralHtmlSupport', StyleUtils, DocumentListStyleSupport ] ); } ); it( 'should register the "style" command', () => { From 949436d15ce447dcb8f885e27c7768d2ea42a155 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 18 Apr 2023 18:37:13 +0200 Subject: [PATCH 08/17] Adding tests. --- packages/ckeditor5-style/src/styleconfig.ts | 1 - packages/ckeditor5-style/src/styleutils.ts | 25 ++- packages/ckeditor5-style/tests/style.js | 38 +++++ packages/ckeditor5-style/tests/styleutils.js | 142 +++++++++++++++++- .../tests/ui/stylegridbuttonview.js | 57 +++++-- .../ckeditor5-style/tests/ui/stylegridview.js | 110 ++++++++++++-- .../tests/ui/stylegroupview.js | 22 ++- .../tests/ui/stylepanelview.js | 99 ++++++++++-- 8 files changed, 449 insertions(+), 45 deletions(-) create mode 100644 packages/ckeditor5-style/tests/style.js diff --git a/packages/ckeditor5-style/src/styleconfig.ts b/packages/ckeditor5-style/src/styleconfig.ts index 850631b15fb..890a7eff7ae 100644 --- a/packages/ckeditor5-style/src/styleconfig.ts +++ b/packages/ckeditor5-style/src/styleconfig.ts @@ -90,5 +90,4 @@ export interface StyleDefinition { name: string; element: string; classes: Array; - previewTemplate?: TemplateDefinition; } diff --git a/packages/ckeditor5-style/src/styleutils.ts b/packages/ckeditor5-style/src/styleutils.ts index b1f3b40e40d..7ce40d741b5 100644 --- a/packages/ckeditor5-style/src/styleutils.ts +++ b/packages/ckeditor5-style/src/styleutils.ts @@ -85,16 +85,23 @@ export default class StyleUtils extends Plugin { } } - if ( !definition.previewTemplate ) { - definition.previewTemplate = this.getStylePreview( definition, [ - { text: 'AaBbCcDdEeFfGgHhIiJj' } - ] ); - } + const previewTemplate = this.getStylePreview( definition, [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] ); if ( modelElements.length ) { - normalizedDefinitions.block.push( { ...definition, modelElements, isBlock: true } ); + normalizedDefinitions.block.push( { + ...definition, + previewTemplate, + modelElements, + isBlock: true + } ); } else { - normalizedDefinitions.inline.push( { ...definition, ghsAttributes } ); + normalizedDefinitions.inline.push( { + ...definition, + previewTemplate, + ghsAttributes + } ); } } @@ -131,6 +138,8 @@ export default class StyleUtils extends Plugin { /** * TODO + * + * @internal */ public getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Array | null { if ( definition.modelElements.includes( block.name ) ) { @@ -197,10 +206,12 @@ export interface NormalizedStyleDefinitions { export interface BlockStyleDefinition extends StyleDefinition { isBlock: true; modelElements: Array; + previewTemplate: TemplateDefinition; } export interface InlineStyleDefinition extends StyleDefinition { ghsAttributes: Array; + previewTemplate: TemplateDefinition; } export type StyleUtilsIsEnabledForBlockEvent = DecoratedMethodEvent; diff --git a/packages/ckeditor5-style/tests/style.js b/packages/ckeditor5-style/tests/style.js new file mode 100644 index 00000000000..b43fd617aad --- /dev/null +++ b/packages/ckeditor5-style/tests/style.js @@ -0,0 +1,38 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; + +import Style from '../src/style'; +import StyleEditing from '../src/styleediting'; +import StyleUI from '../src/styleui'; + +describe( 'Style', () => { + let editor; + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { + plugins: [ Style ] + } ); + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'should be a plugin', () => { + const style = editor.plugins.get( 'Style' ); + + expect( style ).to.instanceOf( Style ); + } ); + + it( 'should be named', () => { + expect( Style.pluginName ).to.equal( 'Style' ); + } ); + + it( 'should require StyleEditing and StyleUI', () => { + expect( Style.requires ).to.deep.equal( [ StyleEditing, StyleUI ] ); + } ); +} ); diff --git a/packages/ckeditor5-style/tests/styleutils.js b/packages/ckeditor5-style/tests/styleutils.js index f8429e0a440..3af4f4aacd0 100644 --- a/packages/ckeditor5-style/tests/styleutils.js +++ b/packages/ckeditor5-style/tests/styleutils.js @@ -8,12 +8,11 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -import Style from '../src/style'; import StyleUtils from '../src/styleutils'; -import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; +import { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; describe( 'StyleUtils', () => { - let editor, element; + let editor, element, styleUtils, dataSchema; testUtils.createSinonSandbox(); @@ -22,8 +21,11 @@ describe( 'StyleUtils', () => { document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ GeneralHtmlSupport, Style ] + plugins: [ StyleUtils, GeneralHtmlSupport ] } ); + + styleUtils = editor.plugins.get( StyleUtils ); + dataSchema = editor.plugins.get( 'DataSchema' ); } ); afterEach( async () => { @@ -36,7 +38,135 @@ describe( 'StyleUtils', () => { expect( StyleUtils.pluginName ).to.equal( 'StyleUtils' ); } ); - it( 'should be loaded by the Style plugin', () => { - expect( editor.plugins.has( 'StyleUtils' ) ).to.be.true; + describe( 'normalizeConfig()', () => { + it( 'should output empty lists for inline and block styles if there is no styles configured', () => { + const styleDefinitions = styleUtils.normalizeConfig( dataSchema ); + + expect( styleDefinitions ).to.deep.equal( { + block: [], + inline: [] + } ); + } ); + + it( 'should normalize block style', () => { + sinon.stub( styleUtils, 'getStylePreview' ).callsFake( definition => ( { + fake: 'preview for ' + definition.name + } ) ); + + const styleDefinitions = styleUtils.normalizeConfig( dataSchema, [ { + name: 'Foo', + element: 'p', + classes: 'bar' + } ] ); + + expect( styleDefinitions ).to.deep.equal( { + block: [ + { + name: 'Foo', + element: 'p', + classes: 'bar', + + isBlock: true, + modelElements: [ + 'paragraph', + 'htmlP' + ], + previewTemplate: { + fake: 'preview for Foo' + } + } + ], + inline: [] + } ); + } ); + + it( 'should normalize inline style', () => { + sinon.stub( styleUtils, 'getStylePreview' ).callsFake( definition => ( { + fake: 'preview for ' + definition.name + } ) ); + + const styleDefinitions = styleUtils.normalizeConfig( dataSchema, [ { + name: 'Bar', + element: 'acronym', + classes: 'foo' + } ] ); + + expect( styleDefinitions ).to.deep.equal( { + inline: [ + { + name: 'Bar', + element: 'acronym', + classes: 'foo', + + ghsAttributes: [ + 'htmlAcronym' + ], + previewTemplate: { + fake: 'preview for Bar' + } + } + ], + block: [] + } ); + } ); + } ); + + describe( 'getStylePreview()', () => { + it( 'should build template definition for style', () => { + const preview = styleUtils.getStylePreview( { + name: 'Foo', + element: 'p', + classes: 'bar' + }, [ { text: 'abc' } ] ); + + expect( preview ).to.deep.equal( { + tag: 'p', + attributes: { + class: 'bar' + }, + children: [ + { text: 'abc' } + ] + } ); + } ); + + it( 'should use passed children', () => { + const children = [ { text: 'abc' } ]; + const preview = styleUtils.getStylePreview( { + name: 'Foo', + element: 'p', + classes: 'bar' + }, children ); + + expect( preview ).to.deep.equal( { + tag: 'p', + attributes: { + class: 'bar' + }, + children: [ + { text: 'abc' } + ] + } ); + + expect( preview.children ).to.equal( children ); + } ); + + it( 'should render non-previewable styles as div', () => { + const preview = styleUtils.getStylePreview( { + name: 'Foo', + element: 'li', + classes: 'bar' + }, [ { text: 'abc' } ] ); + + expect( preview ).to.deep.equal( { + tag: 'div', + attributes: { + class: 'bar' + }, + children: [ + { text: 'abc' } + ] + } ); + } ); } ); } ); diff --git a/packages/ckeditor5-style/tests/ui/stylegridbuttonview.js b/packages/ckeditor5-style/tests/ui/stylegridbuttonview.js index 27ada11695e..9366b1743d4 100644 --- a/packages/ckeditor5-style/tests/ui/stylegridbuttonview.js +++ b/packages/ckeditor5-style/tests/ui/stylegridbuttonview.js @@ -16,7 +16,16 @@ describe( 'StyleGridButtonView', () => { button = new StyleGridButtonView( locale, { name: 'Red heading', element: 'h2', - classes: [ 'red-heading', 'foo' ] + classes: [ 'red-heading', 'foo' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: [ 'red-heading', 'foo' ] + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ); } ); @@ -33,7 +42,16 @@ describe( 'StyleGridButtonView', () => { expect( button.styleDefinition ).to.deep.equal( { name: 'Red heading', element: 'h2', - classes: [ 'red-heading', 'foo' ] + classes: [ 'red-heading', 'foo' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: [ 'red-heading', 'foo' ] + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ); } ); @@ -76,21 +94,40 @@ describe( 'StyleGridButtonView', () => { expect( previewElement.textContent ).to.equal( 'AaBbCcDdEeFfGgHhIiJj' ); } ); - it( 'should render the inner preview as a DIV if non-previewable', () => { + it( 'should render the inner preview based on custom template', () => { const button = new StyleGridButtonView( locale, { - name: 'Non-previewable', - element: 'td', - classes: [ 'a', 'b' ] + name: 'Custom preview', + element: 'li', + classes: [ 'a', 'b' ], + previewTemplate: { + tag: 'ol', + children: [ + { + tag: 'li', + attributes: { + class: [ 'a', 'b' ] + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } + ] + } } ); button.render(); const previewElement = button.previewView.element.firstChild; + const childElement = previewElement.firstChild; - expect( previewElement.tagName ).to.equal( 'DIV' ); - expect( previewElement.classList.contains( 'a' ) ).to.be.true; - expect( previewElement.classList.contains( 'b' ) ).to.be.true; - expect( previewElement.textContent ).to.equal( 'AaBbCcDdEeFfGgHhIiJj' ); + expect( previewElement.tagName ).to.equal( 'OL' ); + expect( previewElement.classList.contains( 'a' ) ).to.be.false; + expect( previewElement.classList.contains( 'b' ) ).to.be.false; + + expect( childElement.tagName ).to.equal( 'LI' ); + expect( childElement.classList.contains( 'a' ) ).to.be.true; + expect( childElement.classList.contains( 'b' ) ).to.be.true; + expect( childElement.textContent ).to.equal( 'AaBbCcDdEeFfGgHhIiJj' ); button.destroy(); } ); diff --git a/packages/ckeditor5-style/tests/ui/stylegridview.js b/packages/ckeditor5-style/tests/ui/stylegridview.js index 057c72cd7e4..b16845a3161 100644 --- a/packages/ckeditor5-style/tests/ui/stylegridview.js +++ b/packages/ckeditor5-style/tests/ui/stylegridview.js @@ -18,12 +18,30 @@ describe( 'StyleGridView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] ); } ); @@ -123,12 +141,30 @@ describe( 'StyleGridView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] ); @@ -150,22 +186,58 @@ describe( 'StyleGridView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Yellow heading', element: 'h2', - classes: [ 'yellow-heading' ] + classes: [ 'yellow-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'yellow-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Green heading', element: 'h2', - classes: [ 'green-heading' ] + classes: [ 'green-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'green-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] ); @@ -220,12 +292,30 @@ describe( 'StyleGridView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] ); diff --git a/packages/ckeditor5-style/tests/ui/stylegroupview.js b/packages/ckeditor5-style/tests/ui/stylegroupview.js index 88f4ce7d03f..edbeba0c83b 100644 --- a/packages/ckeditor5-style/tests/ui/stylegroupview.js +++ b/packages/ckeditor5-style/tests/ui/stylegroupview.js @@ -18,12 +18,30 @@ describe( 'StyleGroupView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] ); } ); diff --git a/packages/ckeditor5-style/tests/ui/stylepanelview.js b/packages/ckeditor5-style/tests/ui/stylepanelview.js index f41a4a95b5a..221b0ff882f 100644 --- a/packages/ckeditor5-style/tests/ui/stylepanelview.js +++ b/packages/ckeditor5-style/tests/ui/stylepanelview.js @@ -22,29 +22,74 @@ describe( 'StylePanelView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ], inline: [ { name: 'Deleted text', element: 'span', - classes: [ 'deleted' ] + classes: [ 'deleted' ], + previewTemplate: { + tag: 'span', + attributes: { + class: 'deleted' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Cited work', element: 'span', - classes: [ 'cited', 'another-class' ] + classes: [ 'cited', 'another-class' ], + previewTemplate: { + tag: 'span', + attributes: { + class: [ 'cited', 'another-class' ] + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } }, { name: 'Small text', element: 'span', - classes: [ 'small' ] + classes: [ 'small' ], + previewTemplate: { + tag: 'span', + attributes: { + class: 'small' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] } ); @@ -111,7 +156,16 @@ describe( 'StylePanelView', () => { { name: 'Deleted text', element: 'span', - classes: [ 'deleted' ] + classes: [ 'deleted' ], + previewTemplate: { + tag: 'span', + attributes: { + class: 'deleted' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] } ); @@ -128,7 +182,16 @@ describe( 'StylePanelView', () => { { name: 'Large heading', element: 'h2', - classes: [ 'large-heading' ] + classes: [ 'large-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'large-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ], inline: [] @@ -295,14 +358,32 @@ describe( 'StylePanelView', () => { { name: 'Red heading', element: 'h2', - classes: [ 'red-heading' ] + classes: [ 'red-heading' ], + previewTemplate: { + tag: 'h2', + attributes: { + class: 'red-heading' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ], inline: [ { name: 'Deleted text', element: 'span', - classes: [ 'deleted' ] + classes: [ 'deleted' ], + previewTemplate: { + tag: 'span', + attributes: { + class: 'deleted' + }, + children: [ + { text: 'AaBbCcDdEeFfGgHhIiJj' } + ] + } } ] } ); From 8ef508fba1a7fbf214243f9df2be1e4d9638da8d Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 18 Apr 2023 18:43:01 +0200 Subject: [PATCH 09/17] Update typings. --- .../ckeditor5-style/src/ui/stylegridbuttonview.ts | 8 ++++---- packages/ckeditor5-style/src/ui/stylegridview.ts | 4 ++-- packages/ckeditor5-style/src/ui/stylegroupview.ts | 12 +++++------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts index bd7b2490166..2db8720b513 100644 --- a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts @@ -10,7 +10,7 @@ import type { Locale } from 'ckeditor5/src/utils'; import { ButtonView, View } from 'ckeditor5/src/ui'; -import type { StyleDefinition } from '../styleconfig'; +import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; /** * A class representing an individual button (style) in the grid. Renders a rich preview of the style. @@ -19,7 +19,7 @@ export default class StyleGridButtonView extends ButtonView { /** * Definition of the style the button will apply when executed. */ - public readonly styleDefinition: StyleDefinition; + public readonly styleDefinition: BlockStyleDefinition | InlineStyleDefinition; /** * The view rendering the preview of the style. @@ -32,7 +32,7 @@ export default class StyleGridButtonView extends ButtonView { * @param locale The localization services instance. * @param styleDefinition Definition of the style. */ - constructor( locale: Locale, styleDefinition: StyleDefinition ) { + constructor( locale: Locale, styleDefinition: BlockStyleDefinition | InlineStyleDefinition ) { super( locale ); this.styleDefinition = styleDefinition; @@ -74,7 +74,7 @@ export default class StyleGridButtonView extends ButtonView { }, children: [ - this.styleDefinition.previewTemplate! + this.styleDefinition.previewTemplate ] } ); diff --git a/packages/ckeditor5-style/src/ui/stylegridview.ts b/packages/ckeditor5-style/src/ui/stylegridview.ts index 876614c9fa8..dc0dd2aec66 100644 --- a/packages/ckeditor5-style/src/ui/stylegridview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridview.ts @@ -11,7 +11,7 @@ import { View, addKeyboardHandlingForGrid, type ViewCollection } from 'ckeditor5 import { FocusTracker, KeystrokeHandler, type Locale } from 'ckeditor5/src/utils'; import StyleGridButtonView from './stylegridbuttonview'; -import type { StyleDefinition } from '../styleconfig'; +import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; import '../../theme/stylegrid.css'; @@ -57,7 +57,7 @@ export default class StyleGridView extends View { * @param locale The localization services instance. * @param styleDefinitions Definitions of the styles. */ - constructor( locale: Locale, styleDefinitions: Array ) { + constructor( locale: Locale, styleDefinitions: Array ) { super( locale ); this.focusTracker = new FocusTracker(); diff --git a/packages/ckeditor5-style/src/ui/stylegroupview.ts b/packages/ckeditor5-style/src/ui/stylegroupview.ts index 60f55f64bf4..f69e9faa915 100644 --- a/packages/ckeditor5-style/src/ui/stylegroupview.ts +++ b/packages/ckeditor5-style/src/ui/stylegroupview.ts @@ -7,15 +7,13 @@ * @module style/ui/stylegroupview */ -import { - LabelView, - View -} from 'ckeditor5/src/ui'; +import { LabelView, View } from 'ckeditor5/src/ui'; +import type { Locale } from 'ckeditor5/src/utils'; + import StyleGridView from './stylegridview'; +import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; import '../../theme/stylegroup.css'; -import type { Locale } from 'ckeditor5/src/utils'; -import type { StyleDefinition } from '../styleconfig'; /** * A class representing a group of styles (e.g. "block" or "inline"). @@ -40,7 +38,7 @@ export default class StyleGroupView extends View { * @param label The localized label of the group. * @param styleDefinitions Definitions of the styles in the group. */ - constructor( locale: Locale, label: string, styleDefinitions: Array ) { + constructor( locale: Locale, label: string, styleDefinitions: Array ) { super( locale ); this.labelView = new LabelView( locale ); From 24fb73da543972dca256de46e26b7e9521532269 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 19 Apr 2023 12:53:39 +0200 Subject: [PATCH 10/17] Added support for multiple view element names to convert to a same model element/attribute. --- .../ckeditor5-html-support/src/datafilter.ts | 5 ++ .../ckeditor5-html-support/src/dataschema.ts | 67 ++++++++++++------- .../src/generalhtmlsupport.ts | 19 ++---- .../src/schemadefinitions.ts | 15 +++++ packages/ckeditor5-style/tests/style.js | 16 +++-- 5 files changed, 82 insertions(+), 40 deletions(-) diff --git a/packages/ckeditor5-html-support/src/datafilter.ts b/packages/ckeditor5-html-support/src/datafilter.ts index ccb2c9911bd..014b048dcb7 100644 --- a/packages/ckeditor5-html-support/src/datafilter.ts +++ b/packages/ckeditor5-html-support/src/datafilter.ts @@ -564,6 +564,11 @@ export default class DataFilter extends Plugin { const conversion = editor.conversion; const attributeKey = definition.model; + // This element is stored in the model as an attribute on a block element, for example DocumentLists. + if ( definition.isBlockAttribute ) { + return; + } + schema.extend( '$text', { allowAttributes: attributeKey } ); diff --git a/packages/ckeditor5-html-support/src/dataschema.ts b/packages/ckeditor5-html-support/src/dataschema.ts index 71ef4a323ea..0c00afd9600 100644 --- a/packages/ckeditor5-html-support/src/dataschema.ts +++ b/packages/ckeditor5-html-support/src/dataschema.ts @@ -49,7 +49,7 @@ export default class DataSchema extends Plugin { /** * A map of registered data schema definitions. */ - private readonly _definitions = new Map(); + private readonly _definitions: Array = []; /** * @inheritDoc @@ -75,14 +75,14 @@ export default class DataSchema extends Plugin { * Add new data schema definition describing block element. */ public registerBlockElement( definition: DataSchemaBlockElementDefinition ): void { - this._definitions.set( definition.model, { ...definition, isBlock: true } ); + this._definitions.push( { ...definition, isBlock: true } ); } /** * Add new data schema definition describing inline element. */ public registerInlineElement( definition: DataSchemaInlineElementDefinition ): void { - this._definitions.set( definition.model, { ...definition, isInline: true } ); + this._definitions.push( { ...definition, isInline: true } ); } /** @@ -134,8 +134,7 @@ export default class DataSchema extends Plugin { * Returns definitions matching the given view name. */ private _getMatchingViewDefinitions( viewName: string | RegExp ): Array { - return Array.from( this._definitions.values() ) - .filter( def => def.view && testViewName( viewName, def.view ) ); + return this._definitions.filter( def => def.view && testViewName( viewName, def.view ) ); } /** @@ -144,21 +143,31 @@ export default class DataSchema extends Plugin { * @param modelName Data schema model name. */ private* _getReferences( modelName: string ): Iterable { - const { modelSchema } = this._definitions.get( modelName )!; + const definitions = this._definitions.filter( definition => definition.model == modelName ); - if ( !modelSchema ) { - return; - } - - const inheritProperties = [ 'inheritAllFrom', 'inheritTypesFrom', 'allowWhere', 'allowContentOf', 'allowAttributesOf' ]; - - for ( const property of inheritProperties ) { - for ( const referenceName of toArray( ( modelSchema as any )[ property ] || [] ) ) { - const definition = this._definitions.get( referenceName ); + for ( const { modelSchema } of definitions ) { + if ( !modelSchema ) { + continue; + } - if ( referenceName !== modelName && definition ) { - yield* this._getReferences( definition.model ); - yield definition; + const inheritProperties = [ + 'inheritAllFrom', + 'inheritTypesFrom', + 'allowWhere', + 'allowContentOf', + 'allowAttributesOf' + ] as const; + + for ( const property of inheritProperties ) { + for ( const referenceName of toArray( modelSchema[ property ] || [] ) ) { + const definitions = this._definitions.filter( definition => definition.model == referenceName ); + + for ( const definition of definitions ) { + if ( referenceName !== modelName ) { + yield* this._getReferences( definition.model ); + yield definition; + } + } } } } @@ -173,13 +182,20 @@ export default class DataSchema extends Plugin { * @param definition Definition update. */ private _extendDefinition( definition: DataSchemaDefinition ): void { - const currentDefinition = this._definitions.get( definition.model ); + const currentDefinitions = Array.from( this._definitions.entries() ) + .filter( ( [ , currentDefinition ] ) => currentDefinition.model == definition.model ); + + if ( currentDefinitions.length == 0 ) { + this._definitions.push( definition ); - const mergedDefinition = mergeWith( {}, currentDefinition, definition, ( target, source ) => { - return Array.isArray( target ) ? target.concat( source ) : undefined; - } ); + return; + } - this._definitions.set( definition.model, mergedDefinition ); + for ( const [ idx, currentDefinition ] of currentDefinitions ) { + this._definitions[ idx ] = mergeWith( {}, currentDefinition, definition, ( target, source ) => { + return Array.isArray( target ) ? target.concat( source ) : undefined; + } ); + } } } @@ -271,4 +287,9 @@ export interface DataSchemaInlineElementDefinition extends DataSchemaDefinition * {@link module:html-support/datafilter~DataFilter#_registerModelPostFixer GHS post-fixer} for more details. */ coupledAttribute?: string; + + /** + * TODO + */ + isBlockAttribute?: boolean; } diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts index 7f58e403fff..f298a15c123 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts @@ -81,22 +81,15 @@ export default class GeneralHtmlSupport extends Plugin { * @param viewElementName A view element name. */ public getGhsAttributeNameForElement( viewElementName: string ): string { - if ( viewElementName == 'ol' || viewElementName == 'ul' ) { - return 'htmlListAttributes'; - } else if ( viewElementName == 'li' ) { - return 'htmlLiAttributes'; - } - const dataSchema = this.editor.plugins.get( 'DataSchema' ); const definitions = Array.from( dataSchema.getDefinitionsForView( viewElementName, false ) ); - if ( - definitions && - definitions.length && - ( definitions[ 0 ] as DataSchemaInlineElementDefinition ).isInline && - !definitions[ 0 ].isObject - ) { - return definitions[ 0 ].model; + const inlineDefinition = definitions.find( definition => ( + ( definition as DataSchemaInlineElementDefinition ).isInline && !definitions[ 0 ].isObject + ) ); + + if ( inlineDefinition ) { + return inlineDefinition.model; } return 'htmlAttributes'; diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.ts b/packages/ckeditor5-html-support/src/schemadefinitions.ts index ebd58c5d540..bb367911988 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.ts +++ b/packages/ckeditor5-html-support/src/schemadefinitions.ts @@ -515,6 +515,21 @@ export default { } ] as Array, inline: [ + { + model: 'htmlLiAttributes', + view: 'li', + isBlockAttribute: true + }, + { + model: 'htmlListAttributes', + view: 'ol', + isBlockAttribute: true + }, + { + model: 'htmlListAttributes', + view: 'ul', + isBlockAttribute: true + }, { model: 'htmlAcronym', view: 'acronym', diff --git a/packages/ckeditor5-style/tests/style.js b/packages/ckeditor5-style/tests/style.js index b43fd617aad..1146039632f 100644 --- a/packages/ckeditor5-style/tests/style.js +++ b/packages/ckeditor5-style/tests/style.js @@ -3,22 +3,30 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +/* global document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; import Style from '../src/style'; import StyleEditing from '../src/styleediting'; import StyleUI from '../src/styleui'; describe( 'Style', () => { - let editor; + let editor, editorElement; beforeEach( async () => { - editor = await VirtualTestEditor.create( { - plugins: [ Style ] + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Style, GeneralHtmlSupport ] } ); } ); afterEach( async () => { + editorElement.remove(); + await editor.destroy(); } ); From 05d7b9edf966f41938604fa30070ff7dfff0239a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 19 Apr 2023 19:23:53 +0200 Subject: [PATCH 11/17] Adding tests. --- .../tests/datafilter.js | 16 ++ .../tests/dataschema.js | 148 ++++++++++++++++++ .../tests/generalhtmlsupport.js | 59 +++++++ 3 files changed, 223 insertions(+) create mode 100644 packages/ckeditor5-html-support/tests/generalhtmlsupport.js diff --git a/packages/ckeditor5-html-support/tests/datafilter.js b/packages/ckeditor5-html-support/tests/datafilter.js index 106642b6cb7..f0bcff79323 100644 --- a/packages/ckeditor5-html-support/tests/datafilter.js +++ b/packages/ckeditor5-html-support/tests/datafilter.js @@ -1315,6 +1315,22 @@ describe( 'DataFilter', () => { editor.getData( '

    foobar

    ' ); } ); + it( 'should not register default converters for isBlockAttribute', () => { + dataSchema.registerInlineElement( { + view: 'xyz', + model: 'htmlXyz', + isBlockAttribute: true + } ); + + dataFilter.allowElement( 'xyz' ); + + editor.setData( '

    foobar

    ' ); + + expect( getModelData( model, { withoutSelection: true } ) ).to.equal( 'foobar' ); + + editor.getData( '

    foobar

    ' ); + } ); + it( 'should use correct priority level for existing features', () => { // 'a' element is registered by data schema with priority 5. // We are checking if this element will be correctly nested due to different diff --git a/packages/ckeditor5-html-support/tests/dataschema.js b/packages/ckeditor5-html-support/tests/dataschema.js index 0c428f60394..8b62fe73ee7 100644 --- a/packages/ckeditor5-html-support/tests/dataschema.js +++ b/packages/ckeditor5-html-support/tests/dataschema.js @@ -41,6 +41,25 @@ describe( 'DataSchema', () => { } ] ); } ); + it( 'should register multiple definitions for the same model attribute', () => { + dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def1' } ); + dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def2' } ); + + const result1 = dataSchema.getDefinitionsForView( 'def1' ); + const result2 = dataSchema.getDefinitionsForView( 'def2' ); + + expect( Array.from( result1 ) ).to.deep.equal( [ { + model: 'htmlDef', + view: 'def1', + isInline: true + } ] ); + expect( Array.from( result2 ) ).to.deep.equal( [ { + model: 'htmlDef', + view: 'def2', + isInline: true + } ] ); + } ); + it( 'should include attribute properties', () => { dataSchema.registerInlineElement( { model: 'htmlDef', @@ -131,6 +150,96 @@ describe( 'DataSchema', () => { expect( Array.from( result ) ).to.deep.equal( getExpectedFakeDefinitions( 'def1' ) ); } ); + it( 'should allow registering multiple view elements with a single model representation', () => { + dataSchema.registerBlockElement( { + view: 'def1', + model: 'htmlDef' + } ); + dataSchema.registerBlockElement( { + view: 'def2', + model: 'htmlDef' + } ); + + const result1 = dataSchema.getDefinitionsForView( 'def1' ); + const result2 = dataSchema.getDefinitionsForView( 'def2' ); + + expect( Array.from( result1 ) ).to.deep.equal( [ + { + isBlock: true, + view: 'def1', + model: 'htmlDef' + } + ] ); + expect( Array.from( result2 ) ).to.deep.equal( [ + { + isBlock: true, + view: 'def2', + model: 'htmlDef' + } + ] ); + } ); + + it( 'should allow registering multiple view elements with a single model representation and dependencies', () => { + dataSchema.registerBlockElement( { + view: 'def1', + model: 'htmlDef', + modelSchema: { + inheritAllFrom: 'htmlBase' + } + } ); + dataSchema.registerBlockElement( { + view: 'def2', + model: 'htmlDef', + modelSchema: { + inheritAllFrom: 'htmlBase' + } + } ); + dataSchema.registerBlockElement( { + model: 'htmlBase', + modelSchema: { + inheritAllFrom: '$block' + } + } ); + + const result1 = dataSchema.getDefinitionsForView( 'def1', true ); + const result2 = dataSchema.getDefinitionsForView( 'def2', true ); + + expect( Array.from( result1 ) ).to.deep.equal( [ + { + isBlock: true, + model: 'htmlBase', + modelSchema: { + inheritAllFrom: '$block' + } + }, + { + isBlock: true, + view: 'def1', + model: 'htmlDef', + modelSchema: { + inheritAllFrom: 'htmlBase' + } + } + ] ); + expect( Array.from( result2 ) ).to.deep.equal( [ + { + isBlock: true, + model: 'htmlBase', + modelSchema: { + inheritAllFrom: '$block' + } + }, + { + isBlock: true, + view: 'def2', + model: 'htmlDef', + modelSchema: { + inheritAllFrom: 'htmlBase' + } + } + ] ); + } ); + it( 'should allow resolving definitions by view name (string)', () => { registerMany( dataSchema, fakeDefinitions ); @@ -235,6 +344,45 @@ describe( 'DataSchema', () => { } ] ); } ); + it( 'should extend schema with new properties (multiple entries for the same model element)', () => { + dataSchema.registerBlockElement( { + view: 'viewName', + model: 'modelName' + } ); + dataSchema.registerBlockElement( { + view: 'viewName2', + model: 'modelName' + } ); + + dataSchema.extendBlockElement( { + model: 'modelName', + paragraphLikeModel: 'htmlDivParagraph', + modelSchema: { + isSelectable: true + } + } ); + + expect( Array.from( dataSchema.getDefinitionsForView( 'viewName' ) ) ).to.deep.equal( [ { + model: 'modelName', + view: 'viewName', + paragraphLikeModel: 'htmlDivParagraph', + modelSchema: { + isSelectable: true + }, + isBlock: true + } ] ); + + expect( Array.from( dataSchema.getDefinitionsForView( 'viewName2' ) ) ).to.deep.equal( [ { + model: 'modelName', + view: 'viewName2', + paragraphLikeModel: 'htmlDivParagraph', + modelSchema: { + isSelectable: true + }, + isBlock: true + } ] ); + } ); + it( 'should append items to array', () => { dataSchema.registerBlockElement( { view: 'viewName', diff --git a/packages/ckeditor5-html-support/tests/generalhtmlsupport.js b/packages/ckeditor5-html-support/tests/generalhtmlsupport.js new file mode 100644 index 00000000000..e77d2cb6146 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/generalhtmlsupport.js @@ -0,0 +1,59 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import { GeneralHtmlSupport } from '../src'; + +describe( 'GeneralHtmlSupport', () => { + let editor, element, dataSchema, generalHtmlSupport; + + beforeEach( async () => { + element = document.createElement( 'div' ); + document.body.appendChild( element ); + + editor = await ClassicTestEditor.create( element, { + plugins: [ GeneralHtmlSupport ] + } ); + + dataSchema = editor.plugins.get( 'DataSchema' ); + generalHtmlSupport = editor.plugins.get( 'GeneralHtmlSupport' ); + } ); + + afterEach( async () => { + element.remove(); + + await editor.destroy(); + } ); + + describe( 'getGhsAttributeNameForElement()', () => { + it( 'should return...', () => { + dataSchema.registerBlockElement( { model: 'def', view: 'def1' } ); + dataSchema.registerBlockElement( { model: 'def', view: 'def2' } ); + dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def3' } ); + dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def4' } ); + dataSchema.registerInlineElement( { model: 'htmlObj', view: 'def5', isObject: true } ); + + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def1' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def2' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def3' ) ).to.equal( 'htmlDef' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def4' ) ).to.equal( 'htmlDef' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def5' ) ).to.equal( 'htmlAttributes' ); + } ); + + it( 'should return....', () => { + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'td' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'th' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ul' ) ).to.equal( 'htmlListAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ol' ) ).to.equal( 'htmlListAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'li' ) ).to.equal( 'htmlLiAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'div' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'p' ) ).to.equal( 'htmlAttributes' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'a' ) ).to.equal( 'htmlA' ); + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'strong' ) ).to.equal( 'htmlStrong' ); + } ); + } ); +} ); From 2008b8880a75da1de522e83fb40a15efa3ee017b Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 20 Apr 2023 20:48:19 +0200 Subject: [PATCH 12/17] Added ability to set style on elements converted to an attribute (figure, tbody, etc). --- .../ckeditor5-html-support/src/datafilter.ts | 60 ++-- .../ckeditor5-html-support/src/dataschema.ts | 14 +- .../src/schemadefinitions.ts | 40 ++- .../tests/datafilter.js | 4 +- .../tests/generalhtmlsupport.js | 23 +- .../tests/integrations/table.js | 310 ++++++++++++++++++ .../tests/documentlist/documentlistutils.js | 5 + packages/ckeditor5-style/src/styleutils.ts | 14 +- .../tests/manual/styledropdown.html | 76 ++--- .../tests/manual/styledropdown.js | 35 +- packages/ckeditor5-style/tests/styleutils.js | 32 ++ 11 files changed, 508 insertions(+), 105 deletions(-) diff --git a/packages/ckeditor5-html-support/src/datafilter.ts b/packages/ckeditor5-html-support/src/datafilter.ts index 014b048dcb7..5c9755653ff 100644 --- a/packages/ckeditor5-html-support/src/datafilter.ts +++ b/packages/ckeditor5-html-support/src/datafilter.ts @@ -208,27 +208,7 @@ export default class DataFilter extends Plugin { */ public allowElement( viewName: string | RegExp ): void { for ( const definition of this._dataSchema.getDefinitionsForView( viewName, true ) ) { - if ( this._allowedElements.has( definition ) ) { - continue; - } - - this._allowedElements.add( definition ); - - // We need to wait for all features to be initialized before we can register - // element, so we can access existing features model schemas. - // If the data has not been initialized yet, _registerElementsAfterInit() method will take care of - // registering elements. - if ( this._dataInitialized ) { - // Defer registration to the next data pipeline data set so any disallow rules could be applied - // even if added after allow rule (disallowElement). - this.editor.data.once( 'set', () => { - this._fireRegisterEvent( definition ); - }, { - // With the highest priority listener we are able to register elements right before - // running data conversion. - priority: priorities.get( 'highest' ) + 1 - } ); - } + this._addAllowedElement( definition ); // Reset cached map to recalculate it on the next usage. this._coupledAttributes = null; @@ -300,6 +280,42 @@ export default class DataFilter extends Plugin { return consumeAttributes( viewElement, conversionApi, this._allowedAttributes ); } + /** + * Adds allowed element definition and fires registration event. + */ + private _addAllowedElement( definition: DataSchemaDefinition ): void { + if ( this._allowedElements.has( definition ) ) { + return; + } + + this._allowedElements.add( definition ); + + // For attribute based integrations (table figure, document lists, etc.) register related element definitions. + if ( 'appliesToBlock' in definition && typeof definition.appliesToBlock == 'string' ) { + for ( const relatedDefinition of this._dataSchema.getDefinitionsForModel( definition.appliesToBlock ) ) { + if ( relatedDefinition.isBlock ) { + this._addAllowedElement( relatedDefinition ); + } + } + } + + // We need to wait for all features to be initialized before we can register + // element, so we can access existing features model schemas. + // If the data has not been initialized yet, _registerElementsAfterInit() method will take care of + // registering elements. + if ( this._dataInitialized ) { + // Defer registration to the next data pipeline data set so any disallow rules could be applied + // even if added after allow rule (disallowElement). + this.editor.data.once( 'set', () => { + this._fireRegisterEvent( definition ); + }, { + // With the highest priority listener we are able to register elements right before + // running data conversion. + priority: priorities.get( 'highest' ) + 1 + } ); + } + } + /** * Registers elements allowed by {@link module:html-support/datafilter~DataFilter#allowElement} method * once {@link module:engine/controller/datacontroller~DataController editor's data controller} is initialized. @@ -565,7 +581,7 @@ export default class DataFilter extends Plugin { const attributeKey = definition.model; // This element is stored in the model as an attribute on a block element, for example DocumentLists. - if ( definition.isBlockAttribute ) { + if ( definition.appliesToBlock ) { return; } diff --git a/packages/ckeditor5-html-support/src/dataschema.ts b/packages/ckeditor5-html-support/src/dataschema.ts index 0c00afd9600..29d91e0e465 100644 --- a/packages/ckeditor5-html-support/src/dataschema.ts +++ b/packages/ckeditor5-html-support/src/dataschema.ts @@ -130,6 +130,13 @@ export default class DataSchema extends Plugin { return definitions; } + /** + * Returns definitions matching the given model name. + */ + public getDefinitionsForModel( modelName: string ): Array { + return this._definitions.filter( definition => definition.model == modelName ); + } + /** * Returns definitions matching the given view name. */ @@ -289,7 +296,10 @@ export interface DataSchemaInlineElementDefinition extends DataSchemaDefinition coupledAttribute?: string; /** - * TODO + * Indicates that element should not be converted as a model text attribute. + * It is used to map view elements that do not have a separate model element but their data is stored in a model attribute. + * For example `    ` element does not have a dedicated model element and GHS stores attributes of `` + * in the `htmlTbodyAttributes` model attribute of the `table` model element. */ - isBlockAttribute?: boolean; + appliesToBlock?: boolean | string; } diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.ts b/packages/ckeditor5-html-support/src/schemadefinitions.ts index bb367911988..61a00b6f622 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.ts +++ b/packages/ckeditor5-html-support/src/schemadefinitions.ts @@ -51,7 +51,7 @@ import type { DataSchemaBlockElementDefinition, DataSchemaInlineElementDefinitio export default { block: [ - // Existing features + // Existing features. { model: 'codeBlock', view: 'pre' @@ -86,11 +86,11 @@ export default { }, { model: 'tableCell', - view: 'th' + view: 'td' }, { model: 'tableCell', - view: 'td' + view: 'th' }, { model: 'tableColumnGroup', @@ -117,7 +117,7 @@ export default { view: 'img' }, - // Compatibility features + // Compatibility features. { model: 'htmlP', view: 'p', @@ -514,22 +514,46 @@ export default { } } ] as Array, + inline: [ + // Existing features (attribute set on an existing model element). { model: 'htmlLiAttributes', view: 'li', - isBlockAttribute: true + appliesToBlock: true }, { model: 'htmlListAttributes', view: 'ol', - isBlockAttribute: true + appliesToBlock: true }, { model: 'htmlListAttributes', view: 'ul', - isBlockAttribute: true + appliesToBlock: true + }, + { + model: 'htmlFigureAttributes', + view: 'figure', + appliesToBlock: 'table' + }, + { + model: 'htmlTheadAttributes', + view: 'thead', + appliesToBlock: 'table' + }, + { + model: 'htmlTbodyAttributes', + view: 'tbody', + appliesToBlock: 'table' }, + { + model: 'htmlFigureAttributes', + view: 'figure', + appliesToBlock: 'imageBlock' + }, + + // Compatibility features. { model: 'htmlAcronym', view: 'acronym', @@ -771,7 +795,7 @@ export default { } }, - // Objects + // Objects. { model: 'htmlObject', view: 'object', diff --git a/packages/ckeditor5-html-support/tests/datafilter.js b/packages/ckeditor5-html-support/tests/datafilter.js index f0bcff79323..2d2a164670e 100644 --- a/packages/ckeditor5-html-support/tests/datafilter.js +++ b/packages/ckeditor5-html-support/tests/datafilter.js @@ -1315,11 +1315,11 @@ describe( 'DataFilter', () => { editor.getData( '

    foobar

    ' ); } ); - it( 'should not register default converters for isBlockAttribute', () => { + it( 'should not register default converters for appliesToBlock', () => { dataSchema.registerInlineElement( { view: 'xyz', model: 'htmlXyz', - isBlockAttribute: true + appliesToBlock: true } ); dataFilter.allowElement( 'xyz' ); diff --git a/packages/ckeditor5-html-support/tests/generalhtmlsupport.js b/packages/ckeditor5-html-support/tests/generalhtmlsupport.js index e77d2cb6146..4e328811ad4 100644 --- a/packages/ckeditor5-html-support/tests/generalhtmlsupport.js +++ b/packages/ckeditor5-html-support/tests/generalhtmlsupport.js @@ -30,28 +30,45 @@ describe( 'GeneralHtmlSupport', () => { } ); describe( 'getGhsAttributeNameForElement()', () => { - it( 'should return...', () => { + beforeEach( () => { dataSchema.registerBlockElement( { model: 'def', view: 'def1' } ); dataSchema.registerBlockElement( { model: 'def', view: 'def2' } ); dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def3' } ); dataSchema.registerInlineElement( { model: 'htmlDef', view: 'def4' } ); dataSchema.registerInlineElement( { model: 'htmlObj', view: 'def5', isObject: true } ); + } ); + it( 'should return "htmlAttributes" for block elements', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def1' ) ).to.equal( 'htmlAttributes' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def2' ) ).to.equal( 'htmlAttributes' ); + } ); + + it( 'should return "htmlAttributes" for inline object elements', () => { + expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def5' ) ).to.equal( 'htmlAttributes' ); + } ); + + it( 'should return model attribute name for inline elements with multiple view representations', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def3' ) ).to.equal( 'htmlDef' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def4' ) ).to.equal( 'htmlDef' ); - expect( generalHtmlSupport.getGhsAttributeNameForElement( 'def5' ) ).to.equal( 'htmlAttributes' ); } ); - it( 'should return....', () => { + it( 'should return model attribute name for block elements with multiple view representations', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'td' ) ).to.equal( 'htmlAttributes' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'th' ) ).to.equal( 'htmlAttributes' ); + } ); + + it( 'should return model attribute name for inline elements with multiple view representations', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ul' ) ).to.equal( 'htmlListAttributes' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ol' ) ).to.equal( 'htmlListAttributes' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'li' ) ).to.equal( 'htmlLiAttributes' ); + } ); + + it( 'should return model attribute name for block elements', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'div' ) ).to.equal( 'htmlAttributes' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'p' ) ).to.equal( 'htmlAttributes' ); + } ); + + it( 'should return model attribute name for inline elements', () => { expect( generalHtmlSupport.getGhsAttributeNameForElement( 'a' ) ).to.equal( 'htmlA' ); expect( generalHtmlSupport.getGhsAttributeNameForElement( 'strong' ) ).to.equal( 'htmlStrong' ); } ); diff --git a/packages/ckeditor5-html-support/tests/integrations/table.js b/packages/ckeditor5-html-support/tests/integrations/table.js index 8bd36d82fcd..c5d6f8de1f6 100644 --- a/packages/ckeditor5-html-support/tests/integrations/table.js +++ b/packages/ckeditor5-html-support/tests/integrations/table.js @@ -370,6 +370,201 @@ describe( 'TableElementSupport', () => { expect( editor.getData() ).to.equal( expectedHtml ); } ); + it( 'should allow enabling only tbody attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: 'tbody', + styles: 'color' + } ] ); + + editor.setData( + '
    ' + + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '2' + + '' + + '' + + '
    ', + attributes: { + 1: { + styles: { + color: 'red' + } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '
    ' + ); + } ); + + it( 'should allow enabling only thead attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: 'thead', + styles: 'color' + } ] ); + + editor.setData( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '
    ' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '2' + + '' + + '' + + '
    ', + attributes: { + 1: { + styles: { + color: 'red' + } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '
    ' + ); + } ); + + it( 'should allow enabling only figure attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: 'figure', + styles: 'color' + } ] ); + + editor.setData( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '
    ' + ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '2' + + '' + + '' + + '
    ', + attributes: { + 1: { + styles: { + color: 'red' + } + } + } + } ); + + expect( editor.getData() ).to.equal( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    1
    2
    ' + + '
    ' + ); + } ); + it( 'should disallow attributes', () => { dataFilter.loadAllowedConfig( [ { name: /^(figure|table|tbody|thead|tr|th|td)$/, @@ -578,6 +773,121 @@ describe( 'TableElementSupport', () => { ); } ); + it( 'should allow attributes modification', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + classes: true + } ] ); + + editor.setData( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    a
    b
    ' + + '
    ' + ); + + model.change( () => { + const htmlSupport = editor.plugins.get( 'GeneralHtmlSupport' ); + const root = editor.model.document.getRoot(); + + htmlSupport.addModelHtmlClass( 'figure', 'added-figure', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'figure', 'foo-figure', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.addModelHtmlClass( 'table', 'added-table', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'table', 'foo-table', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.addModelHtmlClass( 'thead', 'added-thead', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'thead', 'foo-thead', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.addModelHtmlClass( 'tbody', 'added-tbody', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'tbody', 'foo-tbody', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.addModelHtmlClass( 'tr', 'added-tr', root.getNodeByPath( [ 0, 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'tr', 'foo-tr', root.getNodeByPath( [ 0, 0 ] ) ); + htmlSupport.addModelHtmlClass( 'th', 'added-th', root.getNodeByPath( [ 0, 0, 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'th', 'foo-th', root.getNodeByPath( [ 0, 0, 0 ] ) ); + htmlSupport.addModelHtmlClass( 'td', 'added-td', root.getNodeByPath( [ 0, 1, 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'td', 'foo-td', root.getNodeByPath( [ 0, 1, 0 ] ) ); + } ); + + expect( editor.getData() ).to.equal( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    a
    b
    ' + + '
    ' + ); + } ); + + it( 'should allow removing attributes', () => { + dataFilter.loadAllowedConfig( [ { + name: /^(figure|table|tbody|thead|tr|th|td)$/, + classes: true + } ] ); + + editor.setData( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    a
    b
    ' + + '
    ' + ); + + model.change( () => { + const htmlSupport = editor.plugins.get( 'GeneralHtmlSupport' ); + const root = editor.model.document.getRoot(); + + htmlSupport.removeModelHtmlClass( 'figure', 'foobar', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'table', 'foobar', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'thead', 'foobar', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'tbody', 'foobar', root.getNodeByPath( [ 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'tr', 'foobar', root.getNodeByPath( [ 0, 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'th', 'foobar', root.getNodeByPath( [ 0, 0, 0 ] ) ); + htmlSupport.removeModelHtmlClass( 'td', 'foobar', root.getNodeByPath( [ 0, 1, 0 ] ) ); + } ); + + expect( editor.getData() ).to.equal( + '
    ' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
    a
    b
    ' + + '
    ' + ); + } ); + it( 'should disallow styles', () => { dataFilter.loadAllowedConfig( [ { name: /^(figure|table|tbody|thead|tr|th|td)$/, diff --git a/packages/ckeditor5-list/tests/documentlist/documentlistutils.js b/packages/ckeditor5-list/tests/documentlist/documentlistutils.js index f4a5909c54c..f6586c251e0 100644 --- a/packages/ckeditor5-list/tests/documentlist/documentlistutils.js +++ b/packages/ckeditor5-list/tests/documentlist/documentlistutils.js @@ -35,5 +35,10 @@ describe( 'DocumentListUtils', () => { const mock = false; expect( plugin.isListItemBlock( mock ) ).to.be.false; } ); + + it( 'expandListBlocksToCompleteItems', () => { + const mock = []; + expect( plugin.expandListBlocksToCompleteItems( mock ) ).to.be.an( 'array' ); + } ); } ); } ); diff --git a/packages/ckeditor5-style/src/styleutils.ts b/packages/ckeditor5-style/src/styleutils.ts index 7ce40d741b5..2506d77f0dd 100644 --- a/packages/ckeditor5-style/src/styleutils.ts +++ b/packages/ckeditor5-style/src/styleutils.ts @@ -74,12 +74,18 @@ export default class StyleUtils extends Plugin { }; for ( const definition of styleDefinitions ) { - const modelElements = []; - const ghsAttributes = []; + const modelElements: Array = []; + const ghsAttributes: Array = []; for ( const ghsDefinition of dataSchema.getDefinitionsForView( definition.element ) ) { - if ( ghsDefinition.isBlock ) { - modelElements.push( ghsDefinition.model ); + const appliesToBlock = 'appliesToBlock' in ghsDefinition ? ghsDefinition.appliesToBlock : false; + + if ( ghsDefinition.isBlock || appliesToBlock ) { + if ( typeof appliesToBlock == 'string' ) { + modelElements.push( appliesToBlock ); + } else if ( ghsDefinition.isBlock ) { + modelElements.push( ghsDefinition.model ); + } } else { ghsAttributes.push( ghsDefinition.model ); } diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.html b/packages/ckeditor5-style/tests/manual/styledropdown.html index 5c9ebdd8055..794b867013e 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.html +++ b/packages/ckeditor5-style/tests/manual/styledropdown.html @@ -176,6 +176,10 @@ .ck.ck-content li.background-list-item { background-color: #FFAE7F; } + + .ck.ck-content figure.figure-style { + outline: 4px solid #ff0000; + } @@ -184,47 +188,6 @@

    Lots of styles: block + inline

    -

    Top five places to visit this summer

    -

    ...and stay on the budget.

    -
      -
    1. -

      Gran Canaria, Spain

      -

      Gran Canaria is a popular tourist destination. The island is often called “a miniature continent”. This is because it offers a great variety of climates and landscapes, from golden sandy beaches through rocky ravines and white dunes to charming villages. In the mid-2010s, over 3.5 million tourists visited Gran Canaria each year.

      -
      2. -
    2. -

      Florence, Italy

      -

      Florence, established in 59 BC by Julius Caesar himself got its name from two rivers that surrounded the settlement. Thanks to centuries of history, the capital of Tuscany has plenty to offer to tourists hungry for sightseeing. Some of the most interesting places to visit include:

      - -

      Florence is also regarded as one of the top 15 fashion capitals of the world.

      -
      3. -
    3. -

      Porto, Portugal

      -

      Porto, located at the Duero river estuary in northern Portugal, is the country’s second-largest city. It is regarded as one of the oldest European centers, which was recognized in 1996 when deemed the city’s historic center a World Heritage Site.

      -
      - bar -
      Caption
      -
      -

      In 2014 and 2017, Porto was elected The Best European Destination by the Best European Destinations Agency. The city is also home to the Porto School of Architecture, one of the most prestigious architecture schools in the world.

      -
      4. -
    4. -

      Budapest, Hungary

      -

      The capital of Hungary, a spa city sitting across the Danube river, is alive with Roman, Turkish, and European history and cultural heritage. Thermal baths, the picturesque river, and the Gellért Hill citadel are only a few things worth considering while planning your trip to Hungary.

      -
      -

      They said that, of course, Budapest is beautiful. But it is in fact almost ludicrously beautiful. – Anthony Bourdain

      -
      -
      5. -
    5. -

      The Peloponnese, Greece

      -

      The Peloponnese peninsula was already inhabited in prehistoric times. During the bronze age, the first European highly developed culture – the Mycenaean civilization – started there. Roman, Byzantine, Slavic, and Ottoman rules left a significant mark on the place with numerous architectural and cultural relics worth visiting to this day.

      -
      6. -
    - -
    -

    Heading 1

    Paragraph

    Foobar
    @@ -247,14 +210,7 @@

    Heading 1

    Bold Italic Link

    -
      -
    • UL List item 1
      • -
    • UL List item 2
      • -
    -
      -
    1. OL List item 1
      2. -
    2. OL List item 2
      3. -
    +

    Quote

      @@ -263,6 +219,28 @@

      Heading 1

    Quote

    + +
      +
    1. +

      Gran Canaria, Spain

      +

      Gran Canaria is a popular tourist destination. The island is often called “a miniature continent”. This is because it offers + a great variety of climates and landscapes, from golden sandy beaches through rocky ravines and white dunes to charming + villages. In the mid-2010s, over 3.5 million tourists visited Gran Canaria each year.

      +
      2. +
    2. +

      Florence, Italy

      +

      Florence, established in 59 BC by Julius Caesar himself got its name from two rivers that surrounded the settlement. + Thanks to centuries of history, the capital of Tuscany has plenty to offer to tourists hungry for sightseeing. Some of the + most interesting places to visit include:

      +
        +
      • Piazza della Signoria
        • +
      • Mercato Centrale and the San Lorenzo Market
        • +
      • Museum of Zoology and Natural History “La Specola”
        • +
      +

      Florence is also regarded as one of the top 15 fashion capitals of the world.

      +
      3. +
    +

    • Just a few inline styles

    diff --git a/packages/ckeditor5-style/tests/manual/styledropdown.js b/packages/ckeditor5-style/tests/manual/styledropdown.js index 43ad8b3e996..8f5e6f43238 100644 --- a/packages/ckeditor5-style/tests/manual/styledropdown.js +++ b/packages/ckeditor5-style/tests/manual/styledropdown.js @@ -235,6 +235,26 @@ ClassicEditor ...config, style: { definitions: [ + { + name: 'Fancy list', + element: 'ol', + classes: [ 'fancy-list' ] + }, + { + name: 'Italic list', + element: 'ul', + classes: [ 'italic-list' ] + }, + { + name: 'Background list item', + element: 'li', + classes: [ 'background-list-item' ] + }, + { + name: 'Figure', + element: 'figure', + classes: [ 'figure-style' ] + }, { name: 'Red heading', element: 'h2', @@ -335,21 +355,6 @@ ClassicEditor name: 'code.Qux', element: 'code', classes: [ 'Qux' ] - }, - { - name: 'Fancy list', - element: 'ol', - classes: [ 'fancy-list' ] - }, - { - name: 'Italic list', - element: 'ul', - classes: [ 'italic-list' ] - }, - { - name: 'Background list item', - element: 'li', - classes: [ 'background-list-item' ] } ] } diff --git a/packages/ckeditor5-style/tests/styleutils.js b/packages/ckeditor5-style/tests/styleutils.js index 3af4f4aacd0..81a32df5c20 100644 --- a/packages/ckeditor5-style/tests/styleutils.js +++ b/packages/ckeditor5-style/tests/styleutils.js @@ -109,6 +109,38 @@ describe( 'StyleUtils', () => { block: [] } ); } ); + + it( 'should normalize inline style that applies to model element', () => { + sinon.stub( styleUtils, 'getStylePreview' ).callsFake( definition => ( { + fake: 'preview for ' + definition.name + } ) ); + + const styleDefinitions = styleUtils.normalizeConfig( dataSchema, [ { + name: 'Bar', + element: 'figure', + classes: 'foo' + } ] ); + + expect( styleDefinitions ).to.deep.equal( { + block: [ + { + name: 'Bar', + element: 'figure', + classes: 'foo', + isBlock: true, + modelElements: [ + 'htmlFigure', + 'table', + 'imageBlock' + ], + previewTemplate: { + fake: 'preview for Bar' + } + } + ], + inline: [] + } ); + } ); } ); describe( 'getStylePreview()', () => { From 42104744ebbfceaa7c5a92b68559f9d06062b215 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 21 Apr 2023 17:27:44 +0200 Subject: [PATCH 13/17] Review fix. --- .../ckeditor5-html-support/src/dataschema.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/ckeditor5-html-support/src/dataschema.ts b/packages/ckeditor5-html-support/src/dataschema.ts index 29d91e0e465..6996c65ce49 100644 --- a/packages/ckeditor5-html-support/src/dataschema.ts +++ b/packages/ckeditor5-html-support/src/dataschema.ts @@ -150,6 +150,14 @@ export default class DataSchema extends Plugin { * @param modelName Data schema model name. */ private* _getReferences( modelName: string ): Iterable { + const inheritProperties = [ + 'inheritAllFrom', + 'inheritTypesFrom', + 'allowWhere', + 'allowContentOf', + 'allowAttributesOf' + ] as const; + const definitions = this._definitions.filter( definition => definition.model == modelName ); for ( const { modelSchema } of definitions ) { @@ -157,14 +165,6 @@ export default class DataSchema extends Plugin { continue; } - const inheritProperties = [ - 'inheritAllFrom', - 'inheritTypesFrom', - 'allowWhere', - 'allowContentOf', - 'allowAttributesOf' - ] as const; - for ( const property of inheritProperties ) { for ( const referenceName of toArray( modelSchema[ property ] || [] ) ) { const definitions = this._definitions.filter( definition => definition.model == referenceName ); From 9e51c79c1d5337bed0e5f0b6c3260010f6fc08c0 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 21 Apr 2023 19:56:58 +0200 Subject: [PATCH 14/17] Adding tests. --- .../tests/integrations/documentlist.js | 137 ++++++++++++ .../ckeditor5-style/tests/stylecommand.js | 200 +++++++++++++++++- 2 files changed, 331 insertions(+), 6 deletions(-) create mode 100644 packages/ckeditor5-style/tests/integrations/documentlist.js diff --git a/packages/ckeditor5-style/tests/integrations/documentlist.js b/packages/ckeditor5-style/tests/integrations/documentlist.js new file mode 100644 index 00000000000..378465b838d --- /dev/null +++ b/packages/ckeditor5-style/tests/integrations/documentlist.js @@ -0,0 +1,137 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import GeneralHtmlSupport from '@ckeditor/ckeditor5-html-support/src/generalhtmlsupport'; +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import ImageBlock from '@ckeditor/ckeditor5-image/src/imageblock'; +import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption'; +import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote'; +import DocumentList from '@ckeditor/ckeditor5-list/src/documentlist'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; + +import Style from '../../src/style'; + +describe( 'DocumentListStyleSupport', () => { + let editor, editorElement, command, model, doc, root; + + testUtils.createSinonSandbox(); + + beforeEach( async () => { + await createEditor( [ + { + name: 'OL style', + element: 'ol', + classes: [ 'ol-styled' ] + }, + { + name: 'UL style', + element: 'ul', + classes: [ 'ul-styled' ] + }, + { + name: 'LI style', + element: 'li', + classes: [ 'li-styled' ] + } + ] ); + } ); + + afterEach( async () => { + editorElement.remove(); + await editor.destroy(); + } ); + + it( 'should apply OL style to the whole list', () => { + editor.setData( + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '

      • 4

        5

        • ' + + '

      • 6

        7

        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    ' + ); + + model.change( writer => writer.setSelection( root.getChild( 0 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.value ).to.be.empty; + + command.execute( { styleName: 'OL style' } ); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.value ).to.have.members( [ 'OL style' ] ); + + expect( editor.getData() ).to.equal( + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '

      • 4

        5

        • ' + + '

      • 6

        7

        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    ' + ); + + command.execute( { styleName: 'OL style' } ); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.value ).to.be.empty; + + expect( editor.getData() ).to.equal( + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '

      • 4

        5

        • ' + + '

      • 6

        7

        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    ' + ); + } ); + + async function createEditor( styleDefinitions ) { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ + Paragraph, ImageBlock, ImageCaption, Heading, BlockQuote, DocumentList, GeneralHtmlSupport, Style + ], + style: { + definitions: styleDefinitions + } + } ); + + model = editor.model; + command = editor.commands.get( 'style' ); + doc = model.document; + root = doc.getRoot(); + } +} ); diff --git a/packages/ckeditor5-style/tests/stylecommand.js b/packages/ckeditor5-style/tests/stylecommand.js index 95f65c62c1d..00478ad270f 100644 --- a/packages/ckeditor5-style/tests/stylecommand.js +++ b/packages/ckeditor5-style/tests/stylecommand.js @@ -14,6 +14,7 @@ import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption'; import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; import BlockQuote from '@ckeditor/ckeditor5-block-quote/src/blockquote'; import Table from '@ckeditor/ckeditor5-table/src/table'; +import HorizontalLine from '@ckeditor/ckeditor5-horizontal-line/src/horizontalline'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; @@ -104,6 +105,11 @@ describe( 'StyleCommand', () => { name: 'Table style', element: 'table', classes: [ 'example' ] + }, + { + name: 'Figure', + element: 'figure', + classes: [ 'fancy-figure' ] } ]; @@ -197,6 +203,46 @@ describe( 'StyleCommand', () => { ] ); } ); + it( 'should enable styles for the closest widget but no outer blocks', () => { + setData( model, + '
    ' + + '' + + '' + + '[' + + '' + + ']' + + '' + + '
    ' + + '
    ' + ); + + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ + ...blockParagraphStyles.map( ( { name } ) => name ), + ...blockWidgetStyles.map( ( { name } ) => name ) + ] ); + } ); + + it( 'should enable styles for view elements that does not map to model element', () => { + setData( model, + '' + + '' + + '[' + + '' + + ']' + + '' + + '
    ' + ); + + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ + ...blockParagraphStyles.map( ( { name } ) => name ), + ...blockWidgetStyles.map( ( { name } ) => name ) + ] ); + } ); + it( 'should enable styles for the first selected block', () => { setData( model, 'foo' + @@ -290,8 +336,8 @@ describe( 'StyleCommand', () => { } ); describe( '#isEnabled', () => { - it( 'should be disabled if selection is on a block widget', () => { - setData( model, '[]' ); + it( 'should be disabled if none of styles applies to selection', () => { + setData( model, '[]' ); expect( command.isEnabled ).to.be.false; } ); @@ -374,7 +420,7 @@ describe( 'StyleCommand', () => { expect( command.value ).to.have.members( [ 'Vibrant code block' ] ); } ); - it( 'should not detect styles for elements outside a limit element', () => { + it( 'should not detect styles for elements outside a widget element', () => { setData( model, '
    ' + '' + @@ -389,10 +435,54 @@ describe( 'StyleCommand', () => { model.change( writer => { writer.setAttribute( 'htmlAttributes', { classes: [ 'side-quote' ] }, root.getChild( 0 ) ); + writer.setAttribute( 'htmlAttributes', { classes: [ 'example' ] }, root.getNodeByPath( [ 0, 0 ] ) ); writer.setAttribute( 'htmlAttributes', { classes: [ 'red' ] }, root.getNodeByPath( [ 0, 0, 0, 0, 0 ] ) ); } ); - expect( command.value ).to.have.members( [ 'Red paragraph' ] ); + expect( command.value ).to.have.members( [ 'Red paragraph', 'Table style' ] ); + } ); + + it( 'should detect styles for selected widget element only', () => { + setData( model, + '
    ' + + '[
    ' + + '' + + '' + + 'foo' + + '' + + '' + + '
    ]' + + '
    ' + ); + + model.change( writer => { + writer.setAttribute( 'htmlAttributes', { classes: [ 'side-quote' ] }, root.getChild( 0 ) ); + writer.setAttribute( 'htmlAttributes', { classes: [ 'example' ] }, root.getNodeByPath( [ 0, 0 ] ) ); + writer.setAttribute( 'htmlAttributes', { classes: [ 'red' ] }, root.getNodeByPath( [ 0, 0, 0, 0, 0 ] ) ); + } ); + + expect( command.value ).to.have.members( [ 'Table style' ] ); + } ); + + it( 'should detect styles for view elements that does not map to model element', () => { + setData( model, + '' + + '' + + '[' + + '' + + ']' + + '' + + '
    ' + ); + + model.change( writer => { + writer.setAttribute( 'htmlFigureAttributes', { classes: [ 'fancy-figure' ] }, root.getChild( 0 ) ); + writer.setAttribute( 'htmlAttributes', { classes: [ 'example' ] }, root.getChild( 0 ) ); + } ); + + expect( command.value ).to.have.members( [ + ...blockWidgetStyles.map( ( { name } ) => name ) + ] ); } ); } ); @@ -719,7 +809,7 @@ describe( 'StyleCommand', () => { ); } ); - it( 'should add htmlAttribute only to elements in the same limit element', () => { + it( 'should add htmlAttribute only to elements in the same widget element boundaries', () => { setData( model, '
    ' + '' + @@ -751,6 +841,102 @@ describe( 'StyleCommand', () => { ); } ); + it( 'should add htmlAttribute only to elements in the same widget element boundaries (table)', () => { + setData( model, + '
    ' + + '' + + '' + + '
    ' + + '' + + '' + + 'fo[]o' + + '' + + '' + + '
    ' + + '' + + '' + + '' + ); + + command.execute( { styleName: 'Table style' } ); + + expect( getData( model ) ).to.equal( + '' + + '' + + '' + + '
    ' + + '' + + '' + + 'fo[]o' + + '' + + '' + + '
    ' + + '' + + '' + + '' + ); + } ); + + it( 'should add style to view element that does not exist in model', () => { + setData( model, + '' + + '' + + '' + + '[]' + + '' + + '' + + '
    ' + ); + + command.execute( { styleName: 'Figure' } ); + + expect( getData( model ) ).to.equal( + '' + + '' + + '' + + '[]' + + '' + + '' + + '
    ' + ); + } ); + + it( 'should remove style from view element that does not exist in model', () => { + setData( model, + '' + + '' + + '' + + '[]' + + '' + + '' + + '
    ' + ); + + command.execute( { styleName: 'Figure' } ); + + expect( getData( model ) ).to.equal( + '' + + '' + + '' + + '[]' + + '' + + '' + + '
    ' + ); + + command.execute( { styleName: 'Figure' } ); + + expect( getData( model ) ).to.equal( + '' + + '' + + '' + + '[]' + + '' + + '' + + '
    ' + ); + } ); + it( 'should remove htmlAttribute from the selected element', () => { setData( model, 'foo[]bar' ); @@ -812,7 +998,9 @@ describe( 'StyleCommand', () => { document.body.appendChild( editorElement ); editor = await ClassicTestEditor.create( editorElement, { - plugins: [ Paragraph, ImageBlock, ImageCaption, Heading, CodeBlock, BlockQuote, Table, GeneralHtmlSupport, Style ], + plugins: [ + Paragraph, ImageBlock, ImageCaption, Heading, CodeBlock, BlockQuote, Table, HorizontalLine, GeneralHtmlSupport, Style + ], style: { definitions: styleDefinitions }, From da6f3ee309237caea4acae8f420d407da93839c5 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 24 Apr 2023 18:24:40 +0200 Subject: [PATCH 15/17] Added tests. --- .../src/integrations/documentlist.ts | 19 +- .../tests/integrations/documentlist.js | 497 +++++++++++++++++- 2 files changed, 502 insertions(+), 14 deletions(-) diff --git a/packages/ckeditor5-style/src/integrations/documentlist.ts b/packages/ckeditor5-style/src/integrations/documentlist.ts index 5022e3519fe..5d000683798 100644 --- a/packages/ckeditor5-style/src/integrations/documentlist.ts +++ b/packages/ckeditor5-style/src/integrations/documentlist.ts @@ -27,6 +27,7 @@ import type { StyleDefinition } from '../styleconfig'; export default class DocumentListStyleSupport extends Plugin { private _documentListUtils!: DocumentListUtils; private _styleUtils!: StyleUtils; + private _htmlSupport!: GeneralHtmlSupport; /** * @inheritDoc @@ -39,7 +40,7 @@ export default class DocumentListStyleSupport extends Plugin { * @inheritDoc */ public static get requires() { - return [ StyleUtils ] as const; + return [ StyleUtils, 'GeneralHtmlSupport' ] as const; } /** @@ -54,6 +55,7 @@ export default class DocumentListStyleSupport extends Plugin { this._styleUtils = editor.plugins.get( StyleUtils ); this._documentListUtils = this.editor.plugins.get( 'DocumentListUtils' ); + this._htmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); this.listenTo( this._styleUtils, 'isStyleEnabledForBlock', ( evt, [ definition, block ] ) => { if ( this._isStyleEnabledForBlock( definition, block ) ) { @@ -89,11 +91,10 @@ export default class DocumentListStyleSupport extends Plugin { } /** - * TODO + * Verifies if the given style is applicable to the provided block element. */ private _isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { const model = this.editor.model; - const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); if ( ![ 'ol', 'ul', 'li' ].includes( definition.element ) ) { return false; @@ -103,7 +104,7 @@ export default class DocumentListStyleSupport extends Plugin { return false; } - const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); + const attributeName = this._htmlSupport.getGhsAttributeNameForElement( definition.element ); if ( definition.element == 'ol' || definition.element == 'ul' ) { if ( !model.schema.checkAttribute( block, attributeName ) ) { @@ -119,19 +120,17 @@ export default class DocumentListStyleSupport extends Plugin { } /** - * TODO + * Returns true if the given style is applied to the specified block element. */ private _isStyleActiveForBlock( definition: BlockStyleDefinition, block: Element ): boolean { - const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); - - const attributeName = htmlSupport.getGhsAttributeNameForElement( definition.element ); + const attributeName = this._htmlSupport.getGhsAttributeNameForElement( definition.element ); const ghsAttributeValue = block.getAttribute( attributeName ); return this._styleUtils.hasAllClasses( ghsAttributeValue, definition.classes ); } /** - * TODO + * Returns an array of block elements that style should be applied to. */ private _getAffectedBlocks( definition: BlockStyleDefinition, block: Element ): Array | null { if ( !this._isStyleEnabledForBlock( definition, block ) ) { @@ -146,7 +145,7 @@ export default class DocumentListStyleSupport extends Plugin { } /** - * TODO + * Returns a view template definition for the style preview. */ private _getStylePreview( definition: StyleDefinition, children: Iterable ): TemplateDefinition | null { const { element, classes } = definition; diff --git a/packages/ckeditor5-style/tests/integrations/documentlist.js b/packages/ckeditor5-style/tests/integrations/documentlist.js index 378465b838d..e927d128398 100644 --- a/packages/ckeditor5-style/tests/integrations/documentlist.js +++ b/packages/ckeditor5-style/tests/integrations/documentlist.js @@ -38,6 +38,11 @@ describe( 'DocumentListStyleSupport', () => { name: 'LI style', element: 'li', classes: [ 'li-styled' ] + }, + { + name: 'P style', + element: 'p', + classes: [ 'p-styled' ] } ] ); } ); @@ -47,7 +52,491 @@ describe( 'DocumentListStyleSupport', () => { await editor.destroy(); } ); - it( 'should apply OL style to the whole list', () => { + describe( 'enabled styles', () => { + beforeEach( () => { + editor.setData( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
        ' + + '
      1. ' + + '

        10

        ' + + '
        2. ' + + '
      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + ); + } ); + + it( 'OL style should be enabled for OL blocks (selection in the first list block)', () => { + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); + } ); + + it( 'OL style should be enabled for OL blocks (selection in the second block of the first list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 2 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); + } ); + + it( 'OL style should be enabled for OL blocks (selection in the second list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 3 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); + } ); + + it( 'OL style should be disabled for UL blocks (selection in the nested list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 4 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'UL style', 'P style' ] ); + } ); + + it( 'OL style should be enabled for OL blocks (selection in the nested list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 10 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); + } ); + + it( 'OL style should be disabled for non list block', () => { + model.change( writer => writer.setSelection( root.getChild( 0 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'P style' ] ); + } ); + + it( 'UL style should be disabled if htmlListAttributes is disabled', () => { + model.schema.addAttributeCheck( ( context, attributeName ) => { + if ( attributeName == 'htmlListAttributes' ) { + return false; + } + } ); + + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'P style' ] ); + } ); + + it( 'OL style should be disabled if htmlListAttributes is disabled', () => { + model.schema.addAttributeCheck( ( context, attributeName ) => { + if ( attributeName == 'htmlListAttributes' ) { + return false; + } + } ); + + model.change( writer => writer.setSelection( root.getChild( 4 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'P style' ] ); + } ); + + it( 'LI style should be disabled if htmlLiAttributes is disabled', () => { + model.schema.addAttributeCheck( ( context, attributeName ) => { + if ( attributeName == 'htmlLiAttributes' ) { + return false; + } + } ); + + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + + expect( command.enabledStyles ).to.have.members( [ 'OL style', 'P style' ] ); + } ); + } ); + + describe( 'active styles', () => { + beforeEach( () => { + editor.setData( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
        ' + + '
      1. ' + + '

        10

        ' + + '
        2. ' + + '
      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + ); + } ); + + it( 'OL style should be active for OL blocks (selection in the first list block)', () => { + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.have.members( [ 'LI style', 'OL style' ] ); + } ); + + it( 'OL style should be active for OL blocks (selection in the second block of the first list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 2 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.have.members( [ 'LI style', 'OL style' ] ); + } ); + + it( 'OL style should be active for OL blocks (selection in the second list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 3 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.have.members( [ 'OL style' ] ); + } ); + + it( 'UL style should be active for UL blocks (selection in the nested list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 4 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.have.members( [ 'UL style' ] ); + } ); + + it( 'OL style should be enabled for OL blocks (selection in the nested list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 10 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.have.members( [ 'LI style', 'OL style' ] ); + } ); + + it( 'OL style should be disabled for non list block', () => { + model.change( writer => writer.setSelection( root.getChild( 0 ), 0 ) ); + command.refresh(); + + expect( command.value ).to.be.empty; + } ); + } ); + + describe( 'apply style', () => { + beforeEach( () => { + editor.setData( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
        ' + + '
      1. ' + + '

        10

        ' + + '
        2. ' + + '
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'OL style should be applied to the whole list (without sublist)', () => { + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + command.execute( { styleName: 'OL style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
      1. 10
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'OL style should be applied to the closest list (without parent list)', () => { + model.change( writer => writer.setSelection( root.getChild( 10 ), 0 ) ); + command.refresh(); + command.execute( { styleName: 'OL style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
        ' + + '
      1. 10
        2. ' + + '
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'UL style should be applied to the whole list', () => { + model.change( writer => writer.setSelection( root.getChild( 4 ), 0 ) ); + command.refresh(); + command.execute( { styleName: 'UL style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
      1. 10
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'LI style should be applied to the whole list item', () => { + model.change( writer => writer.setSelection( root.getChild( 1 ), 0 ) ); + command.refresh(); + command.execute( { styleName: 'LI style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
      1. 10
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'LI style should be applied to the whole list item (selection in the second block of list item)', () => { + model.change( writer => writer.setSelection( root.getChild( 2 ), 0 ) ); + command.refresh(); + command.execute( { styleName: 'LI style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
      1. 10
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. 13
      2. ' + + '
    ' + ); + } ); + + it( 'style should be applied only to lists', () => { + model.change( writer => { + writer.setSelection( writer.createRange( + writer.createPositionAt( root.getChild( 11 ), 0 ), + writer.createPositionAt( root.getChild( 13 ), 1 ) + ) ); + } ); + + command.refresh(); + command.execute( { styleName: 'OL style' } ); + + expect( editor.getData() ).to.equal( + '

    foo

    ' + + '
      ' + + '
    1. ' + + '

      1

      ' + + '

      2

      ' + + '
      2. ' + + '
    2. ' + + '

      3

      ' + + '
        ' + + '
      • ' + + '

        4

        ' + + '

        5

        ' + + '
        • ' + + '
      • ' + + '

        6

        ' + + '

        7

        ' + + '
        • ' + + '
      ' + + '

      8

      ' + + '
      3. ' + + '
    3. ' + + '

      9

      ' + + '
        ' + + '
      1. 10
        2. ' + + '
      ' + + '

      11

      ' + + '
      4. ' + + '
    ' + + '

    bar

    ' + + '
      ' + + '
    1. ' + + '13' + + '
      2. ' + + '
    ' + ); + } ); + } ); + + it( 'should apply OL style to the whole list and remove it', () => { editor.setData( '
      ' + '
    1. ' + @@ -68,12 +557,12 @@ describe( 'DocumentListStyleSupport', () => { model.change( writer => writer.setSelection( root.getChild( 0 ), 0 ) ); command.refresh(); - expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); expect( command.value ).to.be.empty; command.execute( { styleName: 'OL style' } ); - expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); expect( command.value ).to.have.members( [ 'OL style' ] ); expect( editor.getData() ).to.equal( @@ -95,7 +584,7 @@ describe( 'DocumentListStyleSupport', () => { command.execute( { styleName: 'OL style' } ); - expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style' ] ); + expect( command.enabledStyles ).to.have.members( [ 'LI style', 'OL style', 'P style' ] ); expect( command.value ).to.be.empty; expect( editor.getData() ).to.equal( From e977fa4d6ac473ba49c4557e201ad795ca4ee7a2 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 24 Apr 2023 18:29:08 +0200 Subject: [PATCH 16/17] Removed unused import. --- packages/ckeditor5-style/src/styleconfig.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/ckeditor5-style/src/styleconfig.ts b/packages/ckeditor5-style/src/styleconfig.ts index 890a7eff7ae..2f552553f38 100644 --- a/packages/ckeditor5-style/src/styleconfig.ts +++ b/packages/ckeditor5-style/src/styleconfig.ts @@ -7,8 +7,6 @@ * @module style/styleconfig */ -import type { TemplateDefinition } from 'ckeditor5/src/ui'; - /** * The configuration of the style feature. * From 0a85fcc947f7a4ac2c5124cb2a9226fd2c67f5b3 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Tue, 25 Apr 2023 14:30:31 +0200 Subject: [PATCH 17/17] Review fixes. --- .../src/integrations/documentlist.ts | 8 ++++---- packages/ckeditor5-style/src/stylecommand.ts | 19 ++++++++++--------- packages/ckeditor5-style/src/styleutils.ts | 10 +++++++--- .../src/ui/stylegridbuttonview.ts | 6 +++--- .../ckeditor5-style/src/ui/stylegridview.ts | 4 ++-- .../ckeditor5-style/src/ui/stylegroupview.ts | 4 ++-- 6 files changed, 28 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-style/src/integrations/documentlist.ts b/packages/ckeditor5-style/src/integrations/documentlist.ts index 5d000683798..8ca0fa9ecd7 100644 --- a/packages/ckeditor5-style/src/integrations/documentlist.ts +++ b/packages/ckeditor5-style/src/integrations/documentlist.ts @@ -3,6 +3,10 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ +/** + * @module style/integrations/documentliststylesupport + */ + import { Plugin } from 'ckeditor5/src/core'; import type { Element } from 'ckeditor5/src/engine'; import type { DocumentListUtils } from '@ckeditor/ckeditor5-list'; @@ -20,10 +24,6 @@ import StyleUtils, { import type { StyleDefinition } from '../styleconfig'; -/** - * @module style/integrations/documentliststylesupport - */ - export default class DocumentListStyleSupport extends Plugin { private _documentListUtils!: DocumentListUtils; private _styleUtils!: StyleUtils; diff --git a/packages/ckeditor5-style/src/stylecommand.ts b/packages/ckeditor5-style/src/stylecommand.ts index 9c36ded5a69..dd0203ebe9c 100644 --- a/packages/ckeditor5-style/src/stylecommand.ts +++ b/packages/ckeditor5-style/src/stylecommand.ts @@ -7,19 +7,17 @@ * @module style/stylecommand */ -import type { Element, Schema } from 'ckeditor5/src/engine'; +import type { Element } from 'ckeditor5/src/engine'; import { Command, type Editor } from 'ckeditor5/src/core'; import { logWarning, first } from 'ckeditor5/src/utils'; import type { GeneralHtmlSupport } from '@ckeditor/ckeditor5-html-support'; import StyleUtils, { type BlockStyleDefinition, - type InlineStyleDefinition, + type NormalizedStyleDefinition, type NormalizedStyleDefinitions } from './styleutils'; -type Definition = BlockStyleDefinition | InlineStyleDefinition; - /** * Style command. * @@ -178,13 +176,13 @@ export default class StyleCommand extends Command { const selection = model.document.selection; const htmlSupport: GeneralHtmlSupport = this.editor.plugins.get( 'GeneralHtmlSupport' ); - const allDefinitions: Array = [ + const allDefinitions: Array = [ ...this._styleDefinitions.inline, ...this._styleDefinitions.block ]; const activeDefinitions = allDefinitions.filter( ( { name } ) => this.value.includes( name ) ); - const definition: Definition = allDefinitions.find( ( { name } ) => name == styleName )!; + const definition: NormalizedStyleDefinition = allDefinitions.find( ( { name } ) => name == styleName )!; const shouldAddStyle = forceValue === undefined ? !this.value.includes( definition.name ) : forceValue; model.change( () => { @@ -279,8 +277,11 @@ export default class StyleCommand extends Command { * @param definition Definition whose classes will be compared with all other active definition classes. * @returns Array of classes exclusive to the supplied definition. */ -function getDefinitionExclusiveClasses( activeDefinitions: Array, definition: Definition ): Array { - return activeDefinitions.reduce( ( classes: Array, currentDefinition: Definition ) => { +function getDefinitionExclusiveClasses( + activeDefinitions: Array, + definition: NormalizedStyleDefinition +): Array { + return activeDefinitions.reduce( ( classes: Array, currentDefinition: NormalizedStyleDefinition ) => { if ( currentDefinition.name === definition.name ) { return classes; } @@ -292,6 +293,6 @@ function getDefinitionExclusiveClasses( activeDefinitions: Array, de /** * Checks if provided style definition is of type block. */ -function isBlockStyleDefinition( definition: Definition ): definition is BlockStyleDefinition { +function isBlockStyleDefinition( definition: NormalizedStyleDefinition ): definition is BlockStyleDefinition { return 'isBlock' in definition; } diff --git a/packages/ckeditor5-style/src/styleutils.ts b/packages/ckeditor5-style/src/styleutils.ts index 2506d77f0dd..f77e6cf4b03 100644 --- a/packages/ckeditor5-style/src/styleutils.ts +++ b/packages/ckeditor5-style/src/styleutils.ts @@ -115,7 +115,8 @@ export default class StyleUtils extends Plugin { } /** - * TODO + * Verifies if the given style is applicable to the provided block element. + * * @internal */ public isStyleEnabledForBlock( definition: BlockStyleDefinition, block: Element ): boolean { @@ -131,7 +132,8 @@ export default class StyleUtils extends Plugin { } /** - * TODO + * Returns true if the given style is applied to the specified block element. + * * @internal */ public isStyleActiveForBlock( definition: BlockStyleDefinition, block: Element ): boolean { @@ -143,7 +145,7 @@ export default class StyleUtils extends Plugin { } /** - * TODO + * Returns an array of block elements that style should be applied to. * * @internal */ @@ -220,6 +222,8 @@ export interface InlineStyleDefinition extends StyleDefinition { previewTemplate: TemplateDefinition; } +export type NormalizedStyleDefinition = BlockStyleDefinition | InlineStyleDefinition; + export type StyleUtilsIsEnabledForBlockEvent = DecoratedMethodEvent; export type StyleUtilsIsActiveForBlockEvent = DecoratedMethodEvent; export type StyleUtilsGetAffectedBlocksEvent = DecoratedMethodEvent; diff --git a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts index 2db8720b513..c2918ce5879 100644 --- a/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridbuttonview.ts @@ -10,7 +10,7 @@ import type { Locale } from 'ckeditor5/src/utils'; import { ButtonView, View } from 'ckeditor5/src/ui'; -import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; +import type { NormalizedStyleDefinition } from '../styleutils'; /** * A class representing an individual button (style) in the grid. Renders a rich preview of the style. @@ -19,7 +19,7 @@ export default class StyleGridButtonView extends ButtonView { /** * Definition of the style the button will apply when executed. */ - public readonly styleDefinition: BlockStyleDefinition | InlineStyleDefinition; + public readonly styleDefinition: NormalizedStyleDefinition; /** * The view rendering the preview of the style. @@ -32,7 +32,7 @@ export default class StyleGridButtonView extends ButtonView { * @param locale The localization services instance. * @param styleDefinition Definition of the style. */ - constructor( locale: Locale, styleDefinition: BlockStyleDefinition | InlineStyleDefinition ) { + constructor( locale: Locale, styleDefinition: NormalizedStyleDefinition ) { super( locale ); this.styleDefinition = styleDefinition; diff --git a/packages/ckeditor5-style/src/ui/stylegridview.ts b/packages/ckeditor5-style/src/ui/stylegridview.ts index dc0dd2aec66..888142ad1ba 100644 --- a/packages/ckeditor5-style/src/ui/stylegridview.ts +++ b/packages/ckeditor5-style/src/ui/stylegridview.ts @@ -11,7 +11,7 @@ import { View, addKeyboardHandlingForGrid, type ViewCollection } from 'ckeditor5 import { FocusTracker, KeystrokeHandler, type Locale } from 'ckeditor5/src/utils'; import StyleGridButtonView from './stylegridbuttonview'; -import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; +import type { NormalizedStyleDefinition } from '../styleutils'; import '../../theme/stylegrid.css'; @@ -57,7 +57,7 @@ export default class StyleGridView extends View { * @param locale The localization services instance. * @param styleDefinitions Definitions of the styles. */ - constructor( locale: Locale, styleDefinitions: Array ) { + constructor( locale: Locale, styleDefinitions: Array ) { super( locale ); this.focusTracker = new FocusTracker(); diff --git a/packages/ckeditor5-style/src/ui/stylegroupview.ts b/packages/ckeditor5-style/src/ui/stylegroupview.ts index f69e9faa915..45995c09a1d 100644 --- a/packages/ckeditor5-style/src/ui/stylegroupview.ts +++ b/packages/ckeditor5-style/src/ui/stylegroupview.ts @@ -11,7 +11,7 @@ import { LabelView, View } from 'ckeditor5/src/ui'; import type { Locale } from 'ckeditor5/src/utils'; import StyleGridView from './stylegridview'; -import type { BlockStyleDefinition, InlineStyleDefinition } from '../styleutils'; +import type { NormalizedStyleDefinition } from '../styleutils'; import '../../theme/stylegroup.css'; @@ -38,7 +38,7 @@ export default class StyleGroupView extends View { * @param label The localized label of the group. * @param styleDefinitions Definitions of the styles in the group. */ - constructor( locale: Locale, label: string, styleDefinitions: Array ) { + constructor( locale: Locale, label: string, styleDefinitions: Array ) { super( locale ); this.labelView = new LabelView( locale );