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

fix: character count should includes spoiler text #1535

Merged
merged 2 commits into from
Feb 5, 2023

Conversation

Mini-ghost
Copy link
Member

@Mini-ghost Mini-ghost commented Jan 31, 2023

resolves: #1512

In Mastodon, content warning (spoiler text) will be included in the character count

mastodon

But not currently
main

After this PR, when the content warning (spoiler text) is activated, it will be included in the word count, and it will be ignored when it is closed.

dev.mp4

@stackblitz
Copy link

stackblitz bot commented Jan 31, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jan 31, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit be74572
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/63d8ff87547cf500098c40ba

@netlify
Copy link

netlify bot commented Jan 31, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit be74572
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/63d8ff86547cf500098c40b5
😎 Deploy Preview https://deploy-preview-1535--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Mini-ghost Mini-ghost requested a review from sxzz January 31, 2023 14:15
Comment on lines +29 to +30
if (!draft.params.sensitive)
return
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Suggested change
if (!draft.params.sensitive)
return

Copy link
Member Author

Choose a reason for hiding this comment

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

No, just to make the logic more rigorous, is it recommended to delete it?

Copy link
Member

Choose a reason for hiding this comment

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

I can't really think when this would be called, so I think it probably doesn't matter either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danielroe Got it, Normal operation here would never call. I wish I could remove this if you prefer cleaner code.

@danielroe danielroe merged commit 2bd8dc2 into elk-zone:main Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Character count should include content warning text
2 participants