Skip to content

Commit

Permalink
Add basic remove GHS formatting support for block and container eleme…
Browse files Browse the repository at this point in the history
…nts.
  • Loading branch information
Mati365 committed Sep 11, 2024
1 parent b46c671 commit c1ebbef
Show file tree
Hide file tree
Showing 23 changed files with 421 additions and 24 deletions.
1 change: 1 addition & 0 deletions packages/ckeditor5-code-block/src/converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export function dataViewToModelCodeBlockInsertion(
const language = classesToLanguages[ className ];

if ( language ) {
consumable.consume( viewCodeElement, { classes: [ className ] } );
writer.setAttribute( 'language', language, codeBlock );
break;
}
Expand Down
11 changes: 11 additions & 0 deletions packages/ckeditor5-code-block/tests/codeblockediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,17 @@ describe( 'CodeBlockEditing', () => {
);
} );

it( 'should consume language class during upcast', () => {
const upcastCheck = sinon.spy( ( evt, data, conversionApi ) => {
expect( conversionApi.consumable.test( data.viewItem, { classes: [ 'language-plaintext' ] } ) ).to.be.false;
} );

editor.data.upcastDispatcher.on( 'element:code', upcastCheck, { priority: 'lowest' } );
editor.setData( '<pre><code class="language-plaintext">baz</code></pre>' );

expect( upcastCheck ).to.be.calledOnce;
} );

describe( 'config.codeBlock.languages', () => {
it( 'should be respected when upcasting', () => {
return ClassicTestEditor.create(
Expand Down
36 changes: 35 additions & 1 deletion packages/ckeditor5-engine/src/view/stylesmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,39 @@ export default class StylesMap {
* @param name Style name.
*/
public remove( name: string ): void {
const path = toPath( name );
// Remove of style using lodash's unset and path generated from style name might not work
// if style has custom extractor specified.
//
// For example:
// * `border-top-color` might be extracted from `border.color.top` path.
// * `border-left` uses `getBorderPositionExtractor( 'left' )` which returns `border.left` path.
//
// So if we want to remove `border-top-color` we need to remove `border.color.top` path instead of `border.top.color`.
// We need to use custom extractor that is defined as plain string path to obtain correct path.
// It'll not work for extractors that are functions, however it's not a problem because they are not deterministic
// and might return values generated from different paths.
const extractor = this._styleProcessor.getExtractor( name );
const path = toPath( typeof extractor === 'string' ? extractor : name );

unset( this._styles, path );
delete this._styles[ name ];

this._cleanEmptyObjectsOnPath( path );

// Some styles might be related to others. For example:
//
// * `border-left` is related to `border-left-color`, `border-left-style`, `border-left-width`.
// * `margin` is related to `margin-top`, `margin-right`, `margin-bottom`, `margin-left`.
//
// When removing a style we should also remove all related styles that are longer than the removed one.
// It prevents situations where `.toString()` method would return `margin` style computed from `margin-top`, `margin-right`, ...
// but it's not possible to remove such `margin` style because it's not set directly, so to remove it we need to
// remove all related styles.
for ( const relatedName of this._styleProcessor.getRelatedStyles( name ) ) {
if ( relatedName.length > name.length ) {
this.remove( relatedName );
}
}
}

/**
Expand Down Expand Up @@ -769,6 +796,13 @@ export class StylesProcessor {
this._extractors.set( name, callbackOrPath );
}

/**
* Returns an extractor callback for a style property.
*/
public getExtractor( name: string ): Extractor | undefined {
return this._extractors.get( name );
}

/**
* Adds a reducer callback for a style property.
*
Expand Down
42 changes: 42 additions & 0 deletions packages/ckeditor5-engine/tests/view/stylesmap.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ describe( 'StylesMap', () => {
path: 'foo.top',
value
} ) );

stylesProcessor.setReducer( 'foo', getBoxSidesValueReducer( 'foo' ) );

addBorderRules( stylesProcessor );
addMarginRules( stylesProcessor );

stylesMap = new StylesMap( stylesProcessor );
} );

Expand Down Expand Up @@ -253,6 +256,45 @@ describe( 'StylesMap', () => {

expect( stylesMap.toString() ).to.equal( '' );
} );

it( 'should remove related rules by parent rule', () => {
stylesMap.set( 'border-left-color', 'red' );
stylesMap.set( 'border-left-style', 'solid' );
stylesMap.set( 'border-left-width', '2px' );

expect( stylesMap.toString() ).to.equal( 'border-left:2px solid red;' );

stylesMap.remove( 'border-left' );

expect( stylesMap.toString() ).to.equal( '' );
} );

it( 'should remove multiple css attributes by parent css attribute', () => {
stylesMap.set( 'border', 'orange' );
stylesMap.set( 'border-right-color', 'purple' );
stylesMap.set( 'border-left-color', 'red' );
stylesMap.set( 'border-left-style', 'solid' );
stylesMap.set( 'border-left-width', '2px' );

expect( stylesMap.toString() ).to.equal(
[
'border-bottom-color:orange;',
'border-left:2px solid red;',
'border-right-color:purple;',
'border-top-color:orange;'
].join( '' )
);

stylesMap.remove( 'border-left' );

expect( stylesMap.toString() ).to.equal(
[
'border-bottom-color:orange;',
'border-right-color:purple;',
'border-top-color:orange;'
].join( '' )
);
} );
} );

describe( 'getStyleNames()', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-html-support/src/datafilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {

import {
getHtmlAttributeName,
isGHSAttributeName,
type GHSViewAttributes
} from './utils.js';

Expand Down Expand Up @@ -554,7 +555,7 @@ export default class DataFilter extends Plugin {
}

for ( const attr of change.attributes.keys() ) {
if ( !attr.startsWith( 'html' ) || !attr.endsWith( 'Attributes' ) ) {
if ( !isGHSAttributeName( attr ) ) {
continue;
}

Expand Down
22 changes: 21 additions & 1 deletion packages/ckeditor5-html-support/src/generalhtmlsupport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import ListElementSupport from './integrations/list.js';
import CustomElementSupport from './integrations/customelement.js';
import type { DataSchemaInlineElementDefinition } from './dataschema.js';
import type { DocumentSelection, Item, Model, Range, Selectable } from 'ckeditor5/src/engine.js';
import { getHtmlAttributeName, modifyGhsAttribute } from './utils.js';
import { getHtmlAttributeName, isGHSAttributeName, modifyGhsAttribute, type GHSViewAttributes } from './utils.js';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
import type { GeneralHtmlSupportConfig } from './generalhtmlsupportconfig.js';

Expand Down Expand Up @@ -76,6 +77,25 @@ export default class GeneralHtmlSupport extends Plugin {
dataFilter.loadDisallowedConfig( editor.config.get( 'htmlSupport.disallow' ) || [] );
}

/**
* Iterates over the attributes of a given item or selection and returns a map of GHS attributes.
*
* @internal
* @param item The item or selection to get GHS attributes from.
* @returns A map of GHS attributes.
*/
public getGhsAttributesForElement( item: Item | DocumentSelection ): Map<string, GHSViewAttributes> {
return Array
.from( item.getAttributes() )
.reduce( ( acc, [ attributeName, attributeValue ] ) => {
if ( isGHSAttributeName( attributeName ) ) {
acc.set( attributeName, attributeValue as GHSViewAttributes );
}

return acc;
}, new Map<string, GHSViewAttributes>() );
}

/**
* Returns a GHS model attribute name related to a given view element name.
*
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-html-support/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export { default as DataSchema, type DataSchemaBlockElementDefinition } from './
export { default as HtmlComment } from './htmlcomment.js';
export { default as FullPage } from './fullpage.js';
export { default as HtmlPageDataProcessor } from './htmlpagedataprocessor.js';
export type { GHSViewAttributes } from './utils.js';
export type { GeneralHtmlSupportConfig } from './generalhtmlsupportconfig.js';
export type { default as CodeBlockElementSupport } from './integrations/codeblock.js';
export type { default as CustomElementSupport } from './integrations/customelement.js';
Expand Down
5 changes: 4 additions & 1 deletion packages/ckeditor5-html-support/src/schemadefinitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,10 @@ export default {
model: 'htmlA',
view: 'a',
priority: 5,
coupledAttribute: 'linkHref'
coupledAttribute: 'linkHref',
attributeProperties: {
isFormatting: true
}
},
{
model: 'htmlStrong',
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-html-support/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,18 @@ export function toPascalCase( data: string ): string {
export function getHtmlAttributeName( viewElementName: string ): string {
return `html${ toPascalCase( viewElementName ) }Attributes`;
}

/**
* Checks if the given attribute name is a GHS attribute name.
*
* ```ts
* isGHSAttributeName( 'htmlPAttributes' ); // true
* isGHSAttributeName( 'span' ); // false
* ```
*
* @param name Attribute name to check.
* @returns `true` if the attribute name is a GHS attribute name, `false` otherwise.
*/
export function isGHSAttributeName<S extends string>( name: S ): boolean {
return name.startsWith( 'html' ) && name.endsWith( 'Attributes' );
}
34 changes: 33 additions & 1 deletion packages/ckeditor5-html-support/tests/generalhtmlsupport.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
/* global document */

import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js';
import { GeneralHtmlSupport } from '../src/index.js';

describe( 'GeneralHtmlSupport', () => {
Expand All @@ -16,7 +17,7 @@ describe( 'GeneralHtmlSupport', () => {
document.body.appendChild( element );

editor = await ClassicTestEditor.create( element, {
plugins: [ GeneralHtmlSupport ]
plugins: [ Paragraph, GeneralHtmlSupport ]
} );

dataSchema = editor.plugins.get( 'DataSchema' );
Expand All @@ -29,6 +30,37 @@ describe( 'GeneralHtmlSupport', () => {
await editor.destroy();
} );

describe( 'getGhsAttributesForElement', () => {
let dataFilter;

beforeEach( () => {
dataFilter = editor.plugins.get( 'DataFilter' );
dataFilter.loadAllowedConfig( [ {
name: /^.*$/,
styles: true,
attributes: true,
classes: true
} ] );
} );

it( 'should return map of attributes for block elements', () => {
editor.setData( '<p style="color: red" data-abc="123">foo bar</p>' );

const paragraph = editor.model.document.getRoot().getChild( 0 );
const attributes = generalHtmlSupport.getGhsAttributesForElement( paragraph );

expect( attributes.size ).to.equal( 1 );
expect( attributes.get( 'htmlPAttributes' ) ).to.deep.equal( {
styles: {
color: 'red'
},
attributes: {
'data-abc': '123'
}
} );
} );
} );

describe( 'getGhsAttributeNameForElement()', () => {
beforeEach( () => {
dataSchema.registerBlockElement( { model: 'def', view: 'def1' } );
Expand Down
6 changes: 4 additions & 2 deletions packages/ckeditor5-html-support/tests/manual/codeblock.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold.js';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic.js';
import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough.js';
import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock.js';
import RemoveFormat from '@ckeditor/ckeditor5-remove-format/src/removeformat.js';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin.js';

import GeneralHtmlSupport from '../../src/generalhtmlsupport.js';
Expand Down Expand Up @@ -47,9 +48,10 @@ ClassicEditor
ExtendHTMLSupport,
Italic,
Paragraph,
Strikethrough
Strikethrough,
RemoveFormat
],
toolbar: [ 'codeBlock', '|', 'bold', 'italic', 'strikethrough' ]
toolbar: [ 'codeBlock', '|', 'removeFormat', 'bold', 'italic', 'strikethrough' ]
} )
.then( editor => {
window.editor = editor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ ClassicEditor
toolbar: [
'sourceEditing',
'|',
'RemoveFormat',
'|',
'heading',
'|',
'bold', 'italic', 'strikethrough', 'underline', 'code', 'subscript', 'superscript', 'link',
Expand All @@ -72,9 +74,7 @@ ClassicEditor
'|',
'pageBreak', 'horizontalLine',
'|',
'undo', 'redo',
'|',
'RemoveFormat'
'undo', 'redo'
],
htmlSupport: {
allow: [
Expand Down
9 changes: 7 additions & 2 deletions packages/ckeditor5-html-support/tests/manual/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor.
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset.js';
import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting.js';
import LinkImage from '@ckeditor/ckeditor5-link/src/linkimage.js';
import RemoveFormat from '@ckeditor/ckeditor5-remove-format/src/removeformat.js';

import GeneralHtmlSupport from '../../src/generalhtmlsupport.js';

Expand All @@ -18,9 +19,13 @@ ClassicEditor
ArticlePluginSet,
LinkImage,
SourceEditing,
GeneralHtmlSupport
GeneralHtmlSupport,
RemoveFormat
],
toolbar: [
'sourceEditing', '|', 'removeFormat', '|', 'link', '|', 'heading', '|',
'undo', 'redo', 'bold', 'italic', 'bulletedList', 'numberedList'
],
toolbar: [ 'sourceEditing', '|', 'link', '|', 'heading', '|', 'undo', 'redo', 'bold', 'italic', 'bulletedList', 'numberedList' ],
image: {
toolbar: [
'linkImage', '|',
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-link/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
"@ckeditor/ckeditor5-table": "43.1.0",
"@ckeditor/ckeditor5-theme-lark": "43.1.0",
"@ckeditor/ckeditor5-undo": "43.1.0",
"@ckeditor/ckeditor5-remove-format": "43.1.0",
"@ckeditor/ckeditor5-font": "43.1.0",
"typescript": "5.0.4",
"webpack": "^5.94.0",
"webpack-cli": "^5.1.4"
Expand Down
6 changes: 4 additions & 2 deletions packages/ckeditor5-link/tests/manual/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import Enter from '@ckeditor/ckeditor5-enter/src/enter.js';
import Typing from '@ckeditor/ckeditor5-typing/src/typing.js';
import Link from '../../src/link.js';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js';
import RemoveFormat from '@ckeditor/ckeditor5-remove-format/src/removeformat.js';
import FontColor from '@ckeditor/ckeditor5-font/src/fontcolor.js';
import Undo from '@ckeditor/ckeditor5-undo/src/undo.js';

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ Link, Typing, Paragraph, Undo, Enter ],
toolbar: [ 'link', 'undo', 'redo' ]
plugins: [ Link, Typing, Paragraph, Undo, Enter, RemoveFormat, FontColor ],
toolbar: [ 'link', 'undo', 'redo', 'removeFormat', 'fontColor' ]
} )
.then( editor => {
window.editor = editor;
Expand Down
12 changes: 11 additions & 1 deletion packages/ckeditor5-page-break/src/pagebreakediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,17 @@ export default class PageBreakEditing extends Plugin {
return null;
}

return { name: true };
return {
name: true,
styles: [
...( hasPageBreakBefore ? [ 'page-break-before' ] : [] ),
...( hasPageBreakAfter ? [ 'page-break-after' ] : [] )
],

...element.hasClass( 'page-break' ) && {
classes: [ 'page-break' ]
}
};
},
model: 'pageBreak',

Expand Down
Loading

0 comments on commit c1ebbef

Please sign in to comment.