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: Unassign buttons on the activity/assigned page don't work #559

Conversation

AndrewPrigorshnev
Copy link
Contributor

We've noticed problems when unassigning a topic on the activity/assigned page using this button:

Screenshot 2024-04-04 at 18 59 03

One of two incorrect things happens when pressing this button:

  1. The first topic on the page gets unassigned instead of the selected one
  2. No topics get unassigned (that happens when the first topic on the list is not assigned and only has assigned posts)

That happens because this handler for all buttons somehow always use the same state - the state of the first topic on the page:

onSelect(id) {
switch (id) {
case "unassign":
this.unassign(this.topic.id);
break;
case "reassign":
this.reassign(this.topic, this.assignee);
break;
}
const postId = id.match(/unassign_post_(\d+)/)?.[1];
if (postId) {
this.unassign(postId, "Post");
}
},

There seem to be some nuances in select-kit and I'm not sure whether this should be qualified as a bug in select-kit. However, I've noticed that in other cases when we use DropdownSelectBoxComponent we use the onChange handler rather than onSelect.

So I switched this code to using onChange and that resolved the issue, the onChange handler has access to correct state.

Tests will be in a follow-up.

@jjaffeux
Copy link
Contributor

jjaffeux commented Apr 4, 2024

onSelect is supposed to be deprecated, I guess it didn't get caught.

@AndrewPrigorshnev AndrewPrigorshnev merged commit 0bcd0a1 into main Apr 4, 2024
5 checks passed
@AndrewPrigorshnev AndrewPrigorshnev deleted the fix/unassign-buttons-on-the-user-activity-page-send-wrong-ids branch April 4, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants