Skip to content

Commit

Permalink
Apply suggestions from code review.
Browse files Browse the repository at this point in the history
Co-authored-by: Szymon Cofalik <s.cofalik@cksource.com>
  • Loading branch information
niegowski and scofalik authored Jan 9, 2025
1 parent 08e7d87 commit 0c4253d
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 10 deletions.
5 changes: 3 additions & 2 deletions packages/ckeditor5-engine/src/conversion/viewconsumable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default class ViewConsumable {
private _consumables = new Map<Node | DocumentFragment, ViewElementConsumables | boolean>();

/**
* 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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-engine/src/view/attributeelement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-engine/src/view/element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export default class Element extends Node {
*/
public* getAttributeKeys(): IterableIterator<string> {
// 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';
Expand Down Expand Up @@ -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 ] );
}
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-engine/src/view/stylesmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-engine/src/view/tokenlist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
*/
Expand Down

0 comments on commit 0c4253d

Please sign in to comment.