Skip to content

Commit

Permalink
Merge pull request #103 from ckeditor/i/97-sort-node-attributes
Browse files Browse the repository at this point in the history 
Fix: Node attributes should be sorted in trees and node inspectors (model&view). Closes #97.
  • Loading branch information
@jodator
jodator committed Oct 27, 2020
2 parents e901249 + c507b8c commit d8c109d
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 33 deletions.
25 changes: 16 additions & 9 deletions src/model/data/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -222,7 +223,7 @@ function fillElementDefinition( elementDefinition, ranges ) {

fillElementPositions( elementDefinition, ranges );

elementDefinition.attributes = getNodeAttributes( element );
elementDefinition.attributes = getNodeAttributesForDefinition( element );
}

function fillElementPositions( elementDefinition, ranges ) {
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 15 additions & 8 deletions src/view/data/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -225,7 +226,7 @@ function fillElementDefinition( elementDefinition, ranges ) {

fillViewElementDefinitionPositions( elementDefinition, ranges );

elementDefinition.attributes = getNodeAttrs( element );
elementDefinition.attributes = getNodeAttributesForDefinition( element );
}

function fillViewTextNodeDefinition( textNodeDefinition, ranges ) {
Expand Down Expand Up @@ -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 );
}
113 changes: 98 additions & 15 deletions tests/inspector/model/data/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import {
getEditorModelTreeDefinition,
getEditorModelMarkers,
getEditorModelRanges
getEditorModelRanges,
getEditorModelNodeDefinition
} from '../../../../src/model/data/utils';

import TestEditor from '../../../utils/testeditor';
Expand All @@ -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', () => {
// <paragraph>[]</paragraph>
editor.setData( '<p></p>' );
Expand Down Expand Up @@ -377,5 +378,87 @@ describe( 'model data utils', () => {
}
] );
} );

it( 'should sort node attributes by name', () => {
editor.setData( '<p>a</p>' );

const paragraph = root.getChild( 0 );

// <paragraph>a[]</paragraph>
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( '<p>a</p>' );

const paragraph = root.getChild( 0 );

// <paragraph>a[]</paragraph>
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' ] );
} );
} );
} );
98 changes: 97 additions & 1 deletion tests/inspector/view/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -25,6 +33,8 @@ describe( 'View utils', () => {
plugins: [ Paragraph, BoldEditing ]
} ).then( newEditor => {
editor = newEditor;

root = editor.editing.view.document.getRoot();
} );
} );

Expand All @@ -34,6 +44,92 @@ describe( 'View utils', () => {
return editor.destroy();
} );

describe( 'getEditorViewTreeDefinition()', () => {
it( 'should sort node attributes by name', () => {
editor.setData( '<p>a</p>' );

const paragraph = root.getChild( 0 );

// <p>a[]</p>
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( '<p>a</p>' );

const paragraph = root.getChild( 0 );

// <p>a[]</p>
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 => {
Expand Down

0 comments on commit d8c109d

Please sign in to comment.