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

WIP: make shift+enter work in lists #13546

Merged
merged 3 commits into from
Feb 12, 2019
Merged

WIP: make shift+enter work in lists #13546

merged 3 commits into from
Feb 12, 2019

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jan 28, 2019

Description

Fixes #11215.

How has this been tested?

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.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added the [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable label Jan 28, 2019
@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Feb 7, 2019
@youknowriad
Copy link
Contributor

It seems like indent/outdent buttons are not working properly when I have new lines in list items?

@ellatrix
Copy link
Member Author

@youknowriad Hm, seems to work for me. Maybe it's gone after the rebase? If not, could you provide exact steps?

@youknowriad
Copy link
Contributor

nothing special just use this

<!-- wp:list -->
<ul><li>test<br>sdf<br>sdf</li><li>test</li><li>test</li></ul>
<!-- /wp:list -->

put the caret in the second list item and click the "indent" button.

@youknowriad
Copy link
Contributor

Oh sorry! you're right, it's gone after the rebase.

@youknowriad
Copy link
Contributor

I think, now that we have this PR. We should update the transform from the paragraph to list to avoid creating multiple list items if we have <br> elements in the paragraph block.

@ellatrix
Copy link
Member Author

@youknowriad If I remember correctly that logic was added by intention. I believe someone asked specifically to have it like this, and I kind of agree that it makes sense. Cc @mcsf.

@youknowriad
Copy link
Contributor

Right now if you select three paragraphs, each one with three lines (<br>), and convert them to a list it creates a list with 9 items instead of 3, it doesn't feel right to me personally.

@ellatrix
Copy link
Member Author

@youknowriad Yeah that's true. I think we shouldn't convert on line breaks if multiple paragraphs are selected. If only one is selected, we can create list items from every line. How does that sound?

@youknowriad
Copy link
Contributor

I prefer consistency but it's better yes

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Maybe a small e2e test?

@ellatrix
Copy link
Member Author

Yes, just added it :)

@ellatrix ellatrix added [Package] Rich text /packages/rich-text and removed [Status] In Progress Tracking issues with work in progress labels Feb 12, 2019
@ellatrix ellatrix merged commit 9ef2cf5 into master Feb 12, 2019
@ellatrix ellatrix deleted the try/shift-enter-list branch February 12, 2019 09:56
@youknowriad youknowriad added this to the 5.1 (Gutenberg) milestone Feb 12, 2019
@mcsf
Copy link
Contributor

mcsf commented Feb 15, 2019

Yeah that's true. I think we shouldn't convert on line breaks if multiple paragraphs are selected. If only one is selected, we can create list items from every line. How does that sound?

Sounds good to me, @iseulde.

@ellatrix
Copy link
Member Author

@mcsf Already merged in #13832. :)

youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* WIP: make shift+enter work in lists

* Merge similar logic

* Add e2e tests
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* WIP: make shift+enter work in lists

* Merge similar logic

* Add e2e tests
This was referenced Apr 30, 2020
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 [Package] Rich text /packages/rich-text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be able to make line breaks in list block
4 participants