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: check all in shopping list view #3786

Merged
merged 5 commits into from
Jun 28, 2024

Conversation

ollywelch
Copy link
Contributor

What type of PR is this?

  • feature

What this PR does / why we need it:

This PR adds a new 'check all' button to the button group in the shopping list view. This allows checking all unchecked items in a shopping list, so they can then be deleted using the existing 'delete checked items' button for instance.

image

Which issue(s) this PR fixes:

N/A

Testing

Created a shopping list, added some items, then pressed the check all button. This moved all items from the checked to unchecked state.

@boc-the-git
Copy link
Collaborator

Just reading what the idea is, I think I'd want this to have a confirmation button. It serves an edge case for general usage (IMO), and is fairly destructive if clicked by accident

@ollywelch
Copy link
Contributor Author

Isn't it equally destructive as unchecking all checked items? If you check everything by mistake, you can always uncheck everything again.

@boc-the-git
Copy link
Collaborator

I think there's a good argument for them both to have a confirmation, yes!
Marking as checked is marginally more destructive though, as combined with delete the items are gone. Unchecked doesn't have that.

@michael-genson
Copy link
Collaborator

+1 to putting both of those options behind a confirmation button

@ollywelch
Copy link
Contributor Author

I've added dialogue boxes for:

  • Checking all items
  • Unchecking all items
  • Deleting checked items

Not really sure what I'm doing with the text in the dialogue boxes + translations, please lmk if I've missed something!

@boc-the-git
Copy link
Collaborator

Thanks for taking on that revision @ollywelch, this looks good!

There's a couple scenarios where we probably want a tiny bit more logic though. Right now I can get the popups even though their actions would do nothing, e.g.

  • I can get the "delete all" dialog even though nothing is checked
  • I can get the "uncheck all" dialog even though nothing is check
  • I can get the "check all" dialog even though there's nothing unchecked

Other than that though, functionally this works quite nicely 👍

Copy link
Collaborator

@boc-the-git boc-the-git left a comment

Choose a reason for hiding this comment

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

.. it's forcing me to add a comment as part of changing status 😞

@ollywelch
Copy link
Contributor Author

@boc-the-git thanks for reviewing! I've just added some checks at the start of each open dialogue method, so should only display the prompt when necessary now

Copy link
Collaborator

@boc-the-git boc-the-git left a comment

Choose a reason for hiding this comment

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

Thanks for this! 🚀

@boc-the-git boc-the-git enabled auto-merge (squash) June 28, 2024 09:30
@boc-the-git boc-the-git merged commit 9795b4c into mealie-recipes:mealie-next Jun 28, 2024
11 checks passed
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.

3 participants