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

feat: send upvote / downvote without reason #589

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ydcjeff
Copy link
Contributor

@ydcjeff ydcjeff commented Dec 21, 2024

fix #294

As in the issue description, I have changed to submit the feedback on every upvote / downvote click. The reason dialog stays the same, and it shows Close when there is no input text, Send otherwise.

@coveralls
Copy link

Coverage Status

coverage: 71.672% (-0.06%) from 71.733%
when pulling 126a6a9 on ydcjeff:issue-294
into d5a4be4 on cofacts:master.

variables: {
articleId: articleReply.articleId,
replyId: articleReply.replyId,
vote: value,
Copy link
Member

Choose a reason for hiding this comment

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

This will cause issue in the following user sequence:

  1. The user clicks upvote, fills in some reason and then send
  2. The user clicks upvote again to open popup
  3. The user tries to dismiss the popup

We expect that the user's reason in step 1 should be preserved. However, the current implementation will wipe the reason entirely.

We may argue that the existing feedback button does not load the reason either. However, in this PR, it makes it easier for the user to clear the reason by accident.

I suggest we try amending the reason loading logic before accepting this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if I understand correctly, we want to preserve the user submitted reason when they open the popup?
And optionally create a reply feedback on upvote / downvote with the existing reason if it exists?

Copy link
Member

@MrOrz MrOrz Jan 1, 2025

Choose a reason for hiding this comment

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

Yes -- if we want to submit feedback when popup is opened, it would be great if the reason is preserved. If the user want to change their reason after popup is opened, it's easier for them when the popup is already populated with existing reason.

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.

Send upvote / downvote at the same time button is pressed
3 participants