Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make link toolbar button enabled when text above and link are selected. #17701

Draft
wants to merge 6 commits into
base: ck/epic/17230-linking-experience
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions packages/ckeditor5-engine/src/model/documentselection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
toMap,
uid
} from '@ckeditor/ckeditor5-utils';
import TreeWalker from './treewalker.js';

const storePrefix = 'selection:';

Expand Down Expand Up @@ -1134,20 +1135,28 @@ class LiveSelection extends Selection {
let attrs = null;

if ( !this.isCollapsed ) {
// 1. If selection is a range...
const range = this.getFirstRange();
const shallowRange = new TreeWalker( {
boundaries: this.getFirstRange(),
shallow: true,
ignoreElementEnd: true
} );

// ...look for a first character node in that range and take attributes from it.
for ( const value of range ) {
for ( const value of shallowRange ) {
// If the item is an object, we don't want to get attributes from its children...
if ( value.item.is( 'element' ) && schema.isObject( value.item ) ) {
// ...but collect attributes from inline object.
attrs = getTextAttributes( value.item, schema );
break;
}

if ( value.type == 'text' ) {
attrs = value.item.getAttributes();
}

if ( attrs ) {
attrs = Array.from( attrs );
}

if ( attrs && attrs.length ) {
break;
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-engine/tests/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,7 @@ describe( 'DocumentSelection', () => {
// Step into elements when looking for first character:
selection._setTo( [ new Range( new Position( root, [ 5 ] ), new Position( root, [ 7 ] ) ) ] );

expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [ [ 'd', true ] ] );
expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [] );
} );

it( 'if selection is collapsed it should seek a character to copy that character\'s attributes', () => {
Expand Down Expand Up @@ -1247,10 +1247,10 @@ describe( 'DocumentSelection', () => {
expect( selection.getAttribute( 'bold' ) ).to.equal( true );
} );

it( 'stops reading attributes if selection starts with an object', () => {
it( 'should not stop reading attributes if selection starts with an object', () => {
setData( model, '<p>[<imageBlock></imageBlock><$text bold="true">bar]</$text></p>' );

expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
expect( selection.hasAttribute( 'bold' ) ).to.equal( true );
} );
} );

Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-link/tests/linkcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ describe( 'LinkCommand', () => {
it( 'should be undefined when selection contains not only elements with `linkHref` attribute', () => {
setData( model, 'f[o<$text linkHref="url">ob</$text>]ar' );

expect( command.value ).to.be.undefined;
expect( command.value ).to.equal( 'url' );
} );
} );

Expand Down Expand Up @@ -331,7 +331,7 @@ describe( 'LinkCommand', () => {
it( 'should overwrite existing `linkHref` attribute when selected text wraps text with `linkHref` attribute', () => {
setData( model, 'f[o<$text linkHref="other url">o</$text>ba]r' );

expect( command.value ).to.be.undefined;
expect( command.value ).to.be.equal( 'other url' );

command.execute( 'url' );

Expand Down Expand Up @@ -373,7 +373,7 @@ describe( 'LinkCommand', () => {
'attribute', () => {
setData( model, 'f[o<$text linkHref="other url">ob]a</$text>r' );

expect( command.value ).to.be.undefined;
expect( command.value ).to.be.equal( 'other url' );

command.execute( 'url' );

Expand Down