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

UI for bulk deleting projects #4404

Merged
merged 15 commits into from
Apr 12, 2023

Conversation

magicznyleszek
Copy link
Member

Description

When multiple projects are selected there is an option to delete them all.

Code review notes

Things of interest here:

  • Updated api.ts to throw an error when call fails (or response is not positive)
  • Added disabled and pending option to buttons in KoboPrompt
  • New component ProjectBulkActions that will hold all bulk action buttons (we start with just "Delete")
  • BulkDeletePrompt component for displaying a prompt with checkboxes that needs to be checked before confirming projects deletion

Related issues

Fixes #4376

@magicznyleszek magicznyleszek linked an issue Apr 6, 2023 that may be closed by this pull request
@magicznyleszek magicznyleszek marked this pull request as ready for review April 6, 2023 21:56
@jnm jnm requested a review from LMNTL April 10, 2023 15:53
@jnm jnm assigned LMNTL and unassigned p2edwards Apr 10, 2023
@jnm jnm removed the request for review from p2edwards April 10, 2023 15:53
Copy link
Contributor

@LMNTL LMNTL left a comment

Choose a reason for hiding this comment

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

This looks great! I only encountered one problem in testing: when I select an individual project and delete it, the project count immediately changes to reflect it. But when I bulk delete multiple projects, the project count stays the same until I perform another action.

Video here: https://www.loom.com/share/18bbf2c24a2444f0b0750353ff90a4be

)}
</p>

<Checkbox
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not at all blocking, since it's an upstream issue, but the Checkbox component should probably have a focus state. When tab-navigating, you can't tell which the checkbox is the currently selected element. Would be a small accessibility improvement.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed a small commit that adds focus styles for these :)

Comment on lines +39 to +41
if (!response.ok) {
throw response;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhhh, fetch() error handling 😁

@magicznyleszek
Copy link
Member Author

This looks great! I only encountered one problem in testing: when I select an individual project and delete it, the project count immediately changes to reflect it. But when I bulk delete multiple projects, the project count stays the same until I perform another action.

Video here: https://www.loom.com/share/18bbf2c24a2444f0b0750353ff90a4be

I pushed a hacky fix for that. I left a comment explaining the hack in new code. The old code is a code we definitely aim to remove completely, so I didn't go into much detail in there

@magicznyleszek magicznyleszek merged commit ee63a74 into feature/my-projects Apr 12, 2023
@magicznyleszek magicznyleszek deleted the 4376-bulk-delete-projects branch April 12, 2023 20:25
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.

Bulk operations for projects
4 participants