Unstyle empty list item on Enter key #769
Conversation
|
@ericbiewener This change would be very useful. Any chance you can sign the CLA and we can get someone from Facebook to merge this? |
|
@rylev I signed it a while ago. See the labels on the PR :) |
|
@ericbiewener sorry about that :-D Ok I guess we just need to sit back and wait for someone from Facebook to review this. |
|
Very nice! As with #762 I'd like to get a test for this, but that could be done as a follow-up. I can open an issue for the missing tests. I'll see if we can get this merged. Great PRs! :D |
|
@ericbiewener - looks like it will be useful to merge this in. I know that many Draft-based editors (including one that I wrote) are adding custom logic to handle this case, and it makes sense to me to add it natively in Draft itself. Can you rebase onto master? Then I'll run a few sanity checks on production Facebook & can merge it in. |
|
@niveditc all set. Updated tests as well. |
There was a problem hiding this comment.
Looks good! Just want to be a little more specific about the list item types we're matching like we do in other files :)
Couple of additional comments:
- Looks like you added
package.jsonin the commit with the test - can you exclude that? - I accidentally broke Travis CI on the base rev that you rebased on. Can you rebase again please? :)
|
|
||
| if (!text) { | ||
| const blockType = blockToSplit.getType(); | ||
| if (blockType && blockType.endsWith('-list-item')) { |
There was a problem hiding this comment.
I think we should be specific about the two list item types:
if (blockType === 'unordered-list-item' || blockType === 'ordered-list-item')
There was a problem hiding this comment.
Agreed! All set.
|
It seems that package.json still shows up in this stack & didn't get deleted on d12d823 |
|
@niveditc Sorry, good to go for real this time :) |
|
Next steps are:
|
There was a problem hiding this comment.
niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: **Summary** This appears to be standard functionality in text editors. If the list item has no text and the user pressed `Enter`, rather than creating a new list item, the current one should simply be removed. This is how it works in Google Docs, TextEdit, etc. **Test Plan** 1. Open the Rich Text editor example. 2. Create a list item. Don't add any text to it. 3. Press enter The list item should be removed. Pull Request resolved: facebookarchive#769 Reviewed By: flarnie Differential Revision: D10371552 Pulled By: flarnie fbshipit-source-id: aa975e465c5cdbb1a8def62ba079545836211152
Summary: **Summary** This appears to be standard functionality in text editors. If the list item has no text and the user pressed `Enter`, rather than creating a new list item, the current one should simply be removed. This is how it works in Google Docs, TextEdit, etc. **Test Plan** 1. Open the Rich Text editor example. 2. Create a list item. Don't add any text to it. 3. Press enter The list item should be removed. Pull Request resolved: facebookarchive/draft-js#769 Reviewed By: flarnie Differential Revision: D10371552 Pulled By: flarnie fbshipit-source-id: aa975e465c5cdbb1a8def62ba079545836211152
Summary: **Summary** This appears to be standard functionality in text editors. If the list item has no text and the user pressed `Enter`, rather than creating a new list item, the current one should simply be removed. This is how it works in Google Docs, TextEdit, etc. **Test Plan** 1. Open the Rich Text editor example. 2. Create a list item. Don't add any text to it. 3. Press enter The list item should be removed. Pull Request resolved: facebookarchive/draft-js#769 Reviewed By: flarnie Differential Revision: D10371552 Pulled By: flarnie fbshipit-source-id: aa975e465c5cdbb1a8def62ba079545836211152
Summary
This appears to be standard functionality in text editors. If the list item has no text and the user pressed
Enter, rather than creating a new list item, the current one should simply be removed. This is how it works in Google Docs, TextEdit, etc.Test Plan
The list item should be removed.