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.

Sign up for GitHub

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
Open

feat: send upvote / downvote without reason #589

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion components/ArticleReplyFeedbackControl/ArticleReplyFeedbackControl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,13 @@ function ArticleReplyFeedbackControl({ articleReply, className }) {
const openVotePopover = (event, value) => {
setVotePopoverAnchorEl(event.currentTarget);
setVote(value);
createReplyFeedback({
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 •
edited
Loading

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.

},
});
};

const closeVotePopover = () => {
Expand Down Expand Up @@ -228,7 +235,7 @@ function ArticleReplyFeedbackControl({ articleReply, className }) {
});
}}
>
{t`Send`}
{reason ? t`Send` : t`Close`}
</Button>
</div>
</Popover>
Expand Down
Loading