From 927c680912eac502b97bfbe741b96504df7ba57d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 26 Oct 2020 16:07:52 +0100 Subject: [PATCH] Node attributes should be sorted in trees and node inspectors (model&view). --- src/model/data/utils.js | 25 +++--- src/view/data/utils.js | 23 ++++-- tests/inspector/model/data/utils.js | 113 ++++++++++++++++++++++++---- tests/inspector/view/utils.js | 98 +++++++++++++++++++++++- 4 files changed, 226 insertions(+), 33 deletions(-) diff --git a/src/model/data/utils.js b/src/model/data/utils.js index 66d84c3..eb375a2 100644 --- a/src/model/data/utils.js +++ b/src/model/data/utils.js @@ -162,9 +162,10 @@ export function getEditorModelNodeDefinition( currentEditor, node ) { definition.properties.path = { value: getNodePathString( node ) }; - for ( const [ name, value ] of node.getAttributes() ) { - definition.attributes[ name ] = { value }; - } + getSortedNodeAttributes( node ) + .forEach( ( [ name, value ] ) => { + definition.attributes[ name ] = { value }; + } ); definition.properties = stringifyPropertyList( definition.properties ); definition.attributes = stringifyPropertyList( definition.attributes ); @@ -222,7 +223,7 @@ function fillElementDefinition( elementDefinition, ranges ) { fillElementPositions( elementDefinition, ranges ); - elementDefinition.attributes = getNodeAttributes( element ); + elementDefinition.attributes = getNodeAttributesForDefinition( element ); } function fillElementPositions( elementDefinition, ranges ) { @@ -322,17 +323,23 @@ function fillTextNodeDefinition( textNodeDefinition ) { } } ); - textNodeDefinition.attributes = getNodeAttributes( textNode ); + textNodeDefinition.attributes = getNodeAttributesForDefinition( textNode ); } -function getNodeAttributes( node ) { - const attrs = [ ...node.getAttributes() ].map( ( [ name, value ] ) => { - return [ name, stringify( value, false ) ]; - } ); +function getNodeAttributesForDefinition( node ) { + const attrs = getSortedNodeAttributes( node ) + .map( ( [ name, value ] ) => { + return [ name, stringify( value, false ) ]; + } ); return new Map( attrs ); } +function getSortedNodeAttributes( node ) { + return [ ...node.getAttributes() ] + .sort( ( [ nameA ], [ nameB ] ) => nameA < nameB ? -1 : 1 ); +} + function getRangePositionsInElement( node, range ) { const nodePath = node.path; const startPath = range.start.path; diff --git a/src/view/data/utils.js b/src/view/data/utils.js index 180d56a..069e1e6 100644 --- a/src/view/data/utils.js +++ b/src/view/data/utils.js @@ -114,9 +114,10 @@ export function getEditorViewNodeDefinition( node ) { } } - for ( const [ name, value ] of node.getAttributes() ) { - info.attributes[ name ] = { value }; - } + getSortedNodeAttributes( node ) + .forEach( ( [ name, value ] ) => { + info.attributes[ name ] = { value }; + } ); info.properties = { index: { @@ -225,7 +226,7 @@ function fillElementDefinition( elementDefinition, ranges ) { fillViewElementDefinitionPositions( elementDefinition, ranges ); - elementDefinition.attributes = getNodeAttrs( element ); + elementDefinition.attributes = getNodeAttributesForDefinition( element ); } function fillViewTextNodeDefinition( textNodeDefinition, ranges ) { @@ -344,10 +345,16 @@ function isPathPrefixingAnother( pathA, pathB ) { return false; } -function getNodeAttrs( node ) { - const attrs = [ ...node.getAttributes() ].map( ( [ name, value ] ) => { - return [ name, stringify( value, false ) ]; - } ); +function getNodeAttributesForDefinition( node ) { + const attrs = getSortedNodeAttributes( node ) + .map( ( [ name, value ] ) => { + return [ name, stringify( value, false ) ]; + } ); return new Map( attrs ); } + +function getSortedNodeAttributes( node ) { + return [ ...node.getAttributes() ] + .sort( ( [ nameA ], [ nameB ] ) => nameA.toUpperCase() < nameB.toUpperCase() ? -1 : 1 ); +} diff --git a/tests/inspector/model/data/utils.js b/tests/inspector/model/data/utils.js index 8693227..f8d92cb 100644 --- a/tests/inspector/model/data/utils.js +++ b/tests/inspector/model/data/utils.js @@ -8,7 +8,8 @@ import { getEditorModelTreeDefinition, getEditorModelMarkers, - getEditorModelRanges + getEditorModelRanges, + getEditorModelNodeDefinition } from '../../../../src/model/data/utils'; import TestEditor from '../../../utils/testeditor'; @@ -18,26 +19,26 @@ import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; import { assertTreeItems } from '../../../../tests/utils/utils'; describe( 'model data utils', () => { - describe( 'getEditorModelTreeDefinition()', () => { - let editor, element, root; + let editor, element, root; - beforeEach( () => { - element = document.createElement( 'div' ); - document.body.appendChild( element ); + beforeEach( () => { + element = document.createElement( 'div' ); + document.body.appendChild( element ); - return TestEditor.create( element, { - plugins: [ Paragraph, BoldEditing ] - } ).then( newEditor => { - editor = newEditor; + return TestEditor.create( element, { + plugins: [ Paragraph, BoldEditing ] + } ).then( newEditor => { + editor = newEditor; - root = editor.model.document.getRoot(); - } ); + root = editor.model.document.getRoot(); } ); + } ); - afterEach( () => { - return editor.destroy(); - } ); + afterEach( () => { + return editor.destroy(); + } ); + describe( 'getEditorModelTreeDefinition()', () => { it( 'renders a tree #1', () => { // [] editor.setData( '

' ); @@ -377,5 +378,87 @@ describe( 'model data utils', () => { } ] ); } ); + + it( 'should sort node attributes by name', () => { + editor.setData( '

a

' ); + + const paragraph = root.getChild( 0 ); + + // a[] + editor.model.change( writer => { + writer.setSelection( paragraph, 1 ); + writer.setAttribute( 'b', 'b-value', paragraph ); + writer.setAttribute( 'a', 'a-value', paragraph ); + writer.setAttribute( 'd', 'd-value', paragraph ); + writer.setAttribute( 'c', 'c-value', paragraph ); + } ); + + const ranges = getEditorModelRanges( editor, 'main' ); + const markers = getEditorModelMarkers( editor, 'main' ); + const definition = getEditorModelTreeDefinition( { + currentEditor: editor, + currentRootName: 'main', + ranges, + markers + } ); + + assertTreeItems( definition, [ + { + type: 'element', + name: '$root', + node: root, + attributes: [], + children: [ + { + type: 'element', + name: 'paragraph', + node: root.getChild( 0 ), + attributes: [ + [ 'a', 'a-value' ], + [ 'b', 'b-value' ], + [ 'c', 'c-value' ], + [ 'd', 'd-value' ] + ], + positions: [], + children: [ + { + type: 'text', + node: root.getChild( 0 ).getChild( 0 ), + presentation: { + dontRenderAttributeValue: true + }, + positionsAfter: [ + { offset: 1, isEnd: false, presentation: null, type: 'selection', name: null }, + { offset: 1, isEnd: true, presentation: null, type: 'selection', name: null } + ], + text: 'a' + } + ] + } + ] + } + ] ); + } ); + } ); + + describe( 'getEditorModelNodeDefinition()', () => { + it( 'should sort node attributes by name', () => { + editor.setData( '

a

' ); + + const paragraph = root.getChild( 0 ); + + // a[] + editor.model.change( writer => { + writer.setSelection( paragraph, 1 ); + writer.setAttribute( 'b', 'b-value', paragraph ); + writer.setAttribute( 'a', 'a-value', paragraph ); + writer.setAttribute( 'd', 'd-value', paragraph ); + writer.setAttribute( 'c', 'c-value', paragraph ); + } ); + + const definition = getEditorModelNodeDefinition( editor, paragraph ); + + expect( Object.keys( definition.attributes ) ).to.have.ordered.members( [ 'a', 'b', 'c', 'd' ] ); + } ); } ); } ); diff --git a/tests/inspector/view/utils.js b/tests/inspector/view/utils.js index 687371b..a24fb51 100644 --- a/tests/inspector/view/utils.js +++ b/tests/inspector/view/utils.js @@ -11,8 +11,16 @@ import { nodeToString } from '../../../src/view/utils'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import { + getEditorViewNodeDefinition, + getEditorViewRanges, + getEditorViewTreeDefinition +} from '../../../src/view/data/utils'; + +import { assertTreeItems } from '../../../tests/utils/utils'; + describe( 'View utils', () => { - let editor, element; + let editor, root, element; const container = document.createElement( 'div' ); document.body.appendChild( container ); @@ -25,6 +33,8 @@ describe( 'View utils', () => { plugins: [ Paragraph, BoldEditing ] } ).then( newEditor => { editor = newEditor; + + root = editor.editing.view.document.getRoot(); } ); } ); @@ -34,6 +44,92 @@ describe( 'View utils', () => { return editor.destroy(); } ); + describe( 'getEditorViewTreeDefinition()', () => { + it( 'should sort node attributes by name', () => { + editor.setData( '

a

' ); + + const paragraph = root.getChild( 0 ); + + //

a[]

+ editor.editing.view.change( writer => { + writer.setSelection( paragraph, 1 ); + writer.setAttribute( 'b', 'b-value', paragraph ); + writer.setAttribute( 'a', 'a-value', paragraph ); + writer.setAttribute( 'd', 'd-value', paragraph ); + writer.setAttribute( 'c', 'c-value', paragraph ); + } ); + + const ranges = getEditorViewRanges( editor, 'main' ); + const definition = getEditorViewTreeDefinition( { + currentRootName: 'main', + currentEditor: editor, + ranges + } ); + + assertTreeItems( definition, [ + { + type: 'element', + name: 'div', + node: root, + attributes: [ + [ 'aria-label', 'Rich Text Editor, main' ], + [ 'class', 'ck-blurred ck ck-content ck-editor__editable ck-rounded-corners ck-editor__editable_inline' ], + [ 'contenteditable', 'true' ], + [ 'dir', 'ltr' ], + [ 'lang', 'en' ], + [ 'role', 'textbox' ] + ], + children: [ + { + type: 'element', + name: 'p', + node: root.getChild( 0 ), + attributes: [ + [ 'a', 'a-value' ], + [ 'b', 'b-value' ], + [ 'c', 'c-value' ], + [ 'd', 'd-value' ] + ], + positions: [], + children: [ + { + type: 'text', + node: root.getChild( 0 ).getChild( 0 ), + positionsAfter: [ + { offset: 1, isEnd: false, presentation: null, type: 'selection', name: null }, + { offset: 1, isEnd: true, presentation: null, type: 'selection', name: null } + ], + text: 'a' + } + ] + } + ] + } + ] ); + } ); + } ); + + describe( 'getEditorViewNodeDefinition()', () => { + it( 'should sort node attributes by name', () => { + editor.setData( '

a

' ); + + const paragraph = root.getChild( 0 ); + + //

a[]

+ editor.editing.view.change( writer => { + writer.setSelection( paragraph, 1 ); + writer.setAttribute( 'b', 'b-value', paragraph ); + writer.setAttribute( 'a', 'a-value', paragraph ); + writer.setAttribute( 'd', 'd-value', paragraph ); + writer.setAttribute( 'c', 'c-value', paragraph ); + } ); + + const definition = getEditorViewNodeDefinition( paragraph ); + + expect( Object.keys( definition.attributes ) ).to.have.ordered.members( [ 'a', 'b', 'c', 'd' ] ); + } ); + } ); + describe( 'nodeToString()', () => { it( 'works for AttributeElement', () => { editor.editing.view.change( writer => {