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

Allow link to be edited while selection is collapsed #10372

Merged
merged 2 commits into from
Oct 10, 2018

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Oct 6, 2018

Description

Fixes #10368. Adds logic to applyFormat to edit the format around the caret if it is of the same type. This logic is already present in removeFormat. Adds e2e test.

How has this been tested?

See #10368.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ellatrix ellatrix requested review from a team, noisysocks and talldan October 6, 2018 05:43
@gziolo gziolo added this to the 4.0 milestone Oct 8, 2018
@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable labels Oct 8, 2018
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 👍

expect( result ).toEqual( expected );
expect( result ).not.toBe( record );
expect( getSparseArrayLength( result.formats ) ).toBe( 3 );
} );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if applyFormat is called with no existing format and a collapsed selection? Should we have a unit test for this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then nothing happens, but sure we can have a test for it.

Copy link
Member

@gziolo gziolo Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do it now and merge PR afterward 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this works for you @noisysocks: 9221a33

@gziolo gziolo merged commit 6ff8d3e into master Oct 10, 2018
@gziolo gziolo deleted the fix/edit-link-collapsed branch October 10, 2018 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing a link causes the link url to be inserted into the text
3 participants