From 0c4253dd978cf2852bb9b8fafc683238456053ce Mon Sep 17 00:00:00 2001 From: Kuba Niegowski <1232187+niegowski@users.noreply.github.com> Date: Thu, 9 Jan 2025 17:01:15 +0100 Subject: [PATCH] Apply suggestions from code review. Co-authored-by: Szymon Cofalik --- packages/ckeditor5-engine/src/conversion/viewconsumable.ts | 5 +++-- packages/ckeditor5-engine/src/view/attributeelement.ts | 4 ++-- packages/ckeditor5-engine/src/view/element.ts | 4 ++-- packages/ckeditor5-engine/src/view/stylesmap.ts | 2 +- packages/ckeditor5-engine/src/view/tokenlist.ts | 6 +++--- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/viewconsumable.ts b/packages/ckeditor5-engine/src/conversion/viewconsumable.ts index dfb1c13977a..5bd74a97189 100644 --- a/packages/ckeditor5-engine/src/conversion/viewconsumable.ts +++ b/packages/ckeditor5-engine/src/conversion/viewconsumable.ts @@ -50,7 +50,7 @@ export default class ViewConsumable { private _consumables = new Map(); /** - * Adds {@link module:engine/view/element~Element view element}, {@link module:engine/view/text~Text text node} or + * Adds view {@link module:engine/view/element~Element element}, {@link module:engine/view/text~Text text node} or * {@link module:engine/view/documentfragment~DocumentFragment document fragment} as ready to be consumed. * * ```ts @@ -366,7 +366,7 @@ export class ViewElementConsumables { * * Note: This method accepts only {@link module:engine/conversion/viewconsumable~NormalizedConsumables}. * You can use {@link module:engine/conversion/viewconsumable~normalizeConsumables} helper to convert from - * {@link module:engine/conversion/viewconsumable~Consumables} to NormalizedConsumables. + * {@link module:engine/conversion/viewconsumable~Consumables} to `NormalizedConsumables`. * * Throws {@link module:utils/ckeditorerror~CKEditorError CKEditorError} `viewconsumable-invalid-attribute` when `class` or `style` * attribute is provided - it should be handled separately by providing `style` and `class` in consumables object. @@ -526,6 +526,7 @@ export class ViewElementConsumables { } for ( const [ name, token ] of consumables.attributes ) { + // `value` must be set, because `this.test()` returned `true`. const value = this._attributes.get( name )!; // Plain not-consumed attribute. diff --git a/packages/ckeditor5-engine/src/view/attributeelement.ts b/packages/ckeditor5-engine/src/view/attributeelement.ts index 1716996e96f..04fc7e6be89 100644 --- a/packages/ckeditor5-engine/src/view/attributeelement.ts +++ b/packages/ckeditor5-engine/src/view/attributeelement.ts @@ -170,7 +170,7 @@ export default class AttributeElement extends Element { /** * Used by {@link module:engine/view/element~Element#_mergeAttributesFrom} to verify if the given element can be merged without - * conflicts into the element. + * conflicts into this element. */ protected override _canMergeAttributesFrom( otherElement: AttributeElement ): boolean { // Can't merge if any of elements have an id or a difference of priority. @@ -183,7 +183,7 @@ export default class AttributeElement extends Element { /** * Used by {@link module:engine/view/element~Element#_subtractAttributesOf} to verify if the given element attributes - * can be fully subtracted from the element. + * can be fully subtracted from this element. */ protected override _canSubtractAttributesOf( otherElement: AttributeElement ): boolean { // Can't subtract if any of elements have an id or a difference of priority. diff --git a/packages/ckeditor5-engine/src/view/element.ts b/packages/ckeditor5-engine/src/view/element.ts index f4ada07adce..b3f4dd1f441 100644 --- a/packages/ckeditor5-engine/src/view/element.ts +++ b/packages/ckeditor5-engine/src/view/element.ts @@ -182,6 +182,7 @@ export default class Element extends Node { */ public* getAttributeKeys(): IterableIterator { // This is yielded in this specific order to maintain backward compatibility of data. + // Otherwise, we could simply just have the `for` loop only inside this method. if ( this._classes ) { yield 'class'; @@ -861,8 +862,7 @@ export default class Element extends Node { if ( value !== undefined ) { if ( typeof value == 'string' ) { attributes.push( [ key ] ); - } - else { + } else { for ( const prop of value._getConsumables( token ) ) { attributes.push( [ key, prop ] ); } diff --git a/packages/ckeditor5-engine/src/view/stylesmap.ts b/packages/ckeditor5-engine/src/view/stylesmap.ts index 80a6282ff75..a304a83736e 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.ts +++ b/packages/ckeditor5-engine/src/view/stylesmap.ts @@ -507,7 +507,7 @@ export default class StylesMap implements ElementAttributeValue { * @param attributeKey The attribute name. * @param patternToken The matched style name pattern. * @param patternValue The matched style value pattern. - * @returns An array of tuples [ attributeKey, style name ]. + * @returns An array of tuples `[ attributeKey, styleName ]`. */ public _getTokensMatch( attributeKey: string, diff --git a/packages/ckeditor5-engine/src/view/tokenlist.ts b/packages/ckeditor5-engine/src/view/tokenlist.ts index c7b72cd891e..70f7f45af9a 100644 --- a/packages/ckeditor5-engine/src/view/tokenlist.ts +++ b/packages/ckeditor5-engine/src/view/tokenlist.ts @@ -132,7 +132,7 @@ export default class TokenList implements ElementAttributeValue { * @internal * @param attributeKey The attribute name. * @param patternToken The matched token name pattern. - * @returns An array of tuples [ attributeKey, token ]. + * @returns An array of tuples `[ attributeKey, token ]`. */ public _getTokensMatch( attributeKey: string, @@ -184,8 +184,8 @@ export default class TokenList implements ElementAttributeValue { * Used by {@link module:engine/view/element~Element#_canMergeAttributesFrom} to verify if the given attribute * can be merged without conflicts into the attribute. * - * This method is indirectly used by the {@link module:engine/view/downcastwriter~DowncastWriter} while down-casting - * an {@link module:engine/view/attributeelement~AttributeElement} to merge it with other AttributeElement. + * This method is indirectly used by the {@link module:engine/view/downcastwriter~DowncastWriter} while downcasting + * an {@link module:engine/view/attributeelement~AttributeElement} to merge it with other `AttributeElement`. * * @internal */