Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Add support for preserving text attributes on enter #63

Merged
merged 10 commits into from
Jul 16, 2019
Merged

Add support for preserving text attributes on enter #63

merged 10 commits into from
Jul 16, 2019

Conversation

msamsel
Copy link
Contributor

@msamsel msamsel commented May 28, 2019

@coveralls
Copy link

coveralls commented May 28, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 661e550 on t/40 into 6942320 on master.

tests/entercommand.js Outdated Show resolved Hide resolved
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

Small comments to the code. I'll check the behavior in the meantime as well.

tests/entercommand.js Outdated Show resolved Hide resolved
tests/entercommand.js Outdated Show resolved Hide resolved
src/entercommand.js Outdated Show resolved Hide resolved
src/entercommand.js Outdated Show resolved Hide resolved
@progmancod
Copy link

progmancod commented Jun 26, 2019

Is it possible to set this same behavior to the heading plugin? I mean, copy the element class.

@Reinmar
Copy link
Member

Reinmar commented Jul 1, 2019

Is it possible to set this same behavior to the heading plugin? I mean, copy the element class.

That's a separate piece (subfeature). Here we're copying text attributes. Copying element attributes would be a separate ticket.

@msamsel
Copy link
Contributor Author

msamsel commented Jul 10, 2019

@Mgsy here @jodator already start a review, however, you can also take a look on functionality part. Maybe there exist some not obvious relations and connections, which you might notice.

@Mgsy
Copy link
Member

Mgsy commented Jul 10, 2019

Steps to reproduce

  1. Set the following data: editor.setData("<p>test</p>").
  2. Put the caret at the end of "test".
  3. Activate bold.
  4. Press Shift + Enter.

Current result

The attribute is lost.

GIF

bug_cke5

Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

Small updates to the tests, docs and one API usage.

src/shiftentercommand.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
tests/utils.js Outdated Show resolved Hide resolved
@msamsel msamsel requested a review from jodator July 16, 2019 09:21
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

All is good now. You only missed the selection.getAttributeKeys() use instead of custom keys retrieval from selection.getAttributes().

@jodator jodator merged commit 36bdcd8 into master Jul 16, 2019
@jodator jodator deleted the t/40 branch July 16, 2019 10:09
@jodator jodator restored the t/40 branch July 16, 2019 10:12
@msamsel
Copy link
Contributor Author

msamsel commented Jul 16, 2019

Yeah, I missed this method somehow 🤦‍♂.

@Reinmar Reinmar deleted the t/40 branch July 17, 2019 19:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems with text attributes in newly created lines
6 participants