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

#711 Don't Allow Empty Comments #790

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Demezis
Copy link
Contributor

@Demezis Demezis commented Jun 12, 2022

Description

Issue: #711 Don't Allow Empty Comments

Subtasks

  • I have added this PR to the Journaly Kanban project βœ…
  • I have added validation to post comments
  • I have added validation to threads

Deployment Checklist

  • 🚨 Deploy code to stage
  • 🚨 Deploy code to prod

Screenshots

Animation

@vercel
Copy link

vercel bot commented Jun 12, 2022

@Demezis is attempting to deploy a commit to the Journaly Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@robin-macpherson robin-macpherson left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR @Demezis , it's great to see you contributing again! We're using a ref to track the comment body state for performance reasons and I think the refactor there is not worth the trade-off. To be honest I think for this we can just return early/do nothing if someone attempts to submit a comment and the ref value is empty.

@Demezis
Copy link
Contributor Author

Demezis commented Jul 5, 2022

I have reverted changes regarding threads, and now it should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Frontend work!
Projects
Master Board
  
In progress
🐞 Bug Board
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants