-
Notifications
You must be signed in to change notification settings - Fork 887
feat(site): add batch actions to the workspaces page #9091
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Should we have a test to ensure that only the selected workspaces are sent for delete?
Also, should we list the to-be-deleted workspaces in the confirmation? Just so that the user can be extra sure about what they're doing.
Example: There's a refactor that introduces a bug that doesn't clear the selected items, and suddenly a whole page of previously selected items is deleted. The list would be a last-line-of-defence type of thing.
const confirmDeletion = async () => { | ||
setConfirmError(false) | ||
|
||
if (confirmValue.toLowerCase() !== "delete") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider only accepting the value as stated (i.e. must be upper-case DELETE
) to confirm. But there may be differing opinions on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can test it as it is now on dev.coder.com and see if we get any feedback.
PS: This is under the experimental flag!
Screen.Recording.2023-08-14.at.16.15.48.mov