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

Editable: Fix splitting inline Editables using shift+enter #1299

Merged
merged 5 commits into from Jun 20, 2017

Conversation

youknowriad
Copy link
Contributor

closes #1243 #1254

In this PR, I'm proposing to split a text block if we hit shift+enter.
The current shift+enter behaviour is broken (see #1243 and #1254)

@youknowriad youknowriad added [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 labels Jun 20, 2017
@youknowriad youknowriad self-assigned this Jun 20, 2017
@ellatrix
Copy link
Member

So, enter behaves as it does now, shift+enter splits is right away?

@jasmussen
Copy link
Contributor

I think I like this! Nice 👍 👍

@youknowriad
Copy link
Contributor Author

@iseulde yes, that's the idea.

@ellatrix
Copy link
Member

Testing

@ellatrix
Copy link
Member

@youknowriad I changed it slightly so shift+enter in other inline Editables doesn't do anything. Atm it's broken there as well.

@youknowriad
Copy link
Contributor Author

@iseulde nice, thanks

@@ -206,6 +206,14 @@ export default class Editable extends wp.element.Component {
event.preventDefault();
event.stopImmediatePropagation();
}

if ( event.keyCode === ENTER && event.shiftKey && this.props.inline ) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we include some more inline documentation describing what these various inline conditions are achieving? Seems like they'll be difficult to revisit in the future otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Adding some.

@ellatrix
Copy link
Member

There's another change that this PR is introducing: when you split with just enter, and merge the blocks back, there's a line break left over, whereas before it would merge without any line breaks and the caret at the place they are merged.

@youknowriad
Copy link
Contributor Author

@iseulde Merging back without empty lines should be fixed now.

@@ -207,6 +207,8 @@ export default class Editable extends wp.element.Component {
event.stopImmediatePropagation();
}

// If we click shift+Enter on inline Editables, we should avoid creating `p` tags
Copy link
Member

Choose a reason for hiding this comment

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

It would also split two contenteditables into two, e.g. a heading will suddenly become two headings.

@ellatrix
Copy link
Member

Works well in various browsers.
Seeing another small inconsistency: pressing enter with an uncollapsed selection will delete the selection, while pressing shift+enter will delete and split it. Not sure what the right behaviour should be.

@youknowriad youknowriad merged commit 8fd70f4 into master Jun 20, 2017
@youknowriad youknowriad deleted the fix/split-text-behaviour branch June 20, 2017 13:32
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.

Text block: Failures when using shift+enter for adding line breaks
4 participants