Skip to content

Wordcount: Return 0 if text is empty#10602

Merged
Soean merged 8 commits intomasterfrom
update/wordcount-no-words
Oct 25, 2018
Merged

Wordcount: Return 0 if text is empty#10602
Soean merged 8 commits intomasterfrom
update/wordcount-no-words

Conversation

@Soean
Copy link
Copy Markdown
Member

@Soean Soean commented Oct 14, 2018

Description

Wordcount should return 0 if text is empty.

Screenshots

before
bildschirmfoto 2018-10-14 um 23 19 06

after
bildschirmfoto 2018-10-14 um 23 23 12

@Soean Soean added the [Package] Word count /packages/wordcount label Oct 14, 2018
Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

While I totally agree that for empty strings ('') we should return 0, I'm not sure if we should return 0 if the text is null or undefined. Maybe we can stay on the safe side and just return 0 for empty strings keeping the undefined return for other "falsy" values.

@Soean
Copy link
Copy Markdown
Member Author

Soean commented Oct 25, 2018

@jorgefilipecosta
Thanks for your review. I changed the check and added a test.

Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for iterating on this!

@Soean Soean merged commit 55882b3 into master Oct 25, 2018
@Soean Soean added this to the 4.2 milestone Oct 25, 2018
@Soean Soean deleted the update/wordcount-no-words branch October 25, 2018 12:22
antpb pushed a commit to antpb/gutenberg that referenced this pull request Oct 26, 2018
* Return 0 if text is empty

* return 0 with empty stirng

* return 0 for enpty string

* small typo

* Add message to changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Word count /packages/wordcount

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants