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

Ap sm delete item #33

Merged
merged 4 commits into from
Mar 17, 2024
Merged

Ap sm delete item #33

merged 4 commits into from
Mar 17, 2024

Conversation

andiedoescode
Copy link
Collaborator

@andiedoescode andiedoescode commented Mar 13, 2024

Description

  • Added delete button next to each ListItem.
  • deleteHandler function for the buttom to trigger user confirmation window, then the deleteItem function.
  • Completed deleteItem function in firebase.js

Note: While completing this issue, lead to potential creation of new tickets (deleting lists, user feedback/error handling UI)

Related Issue

Closes #12

Acceptance Criteria

  • The ListItem component renders a button that allows the user to delete an item from their list when clicked
  • Clicking the delete button prompts the user to confirm that they really want to delete the item
  • The deleteItem function in api/firebase.js has been filled out, and deletes the item from the Firestore database

Type of Changes

new feature

Updates

Before

image

After

image
image
image

Testing Steps / QA Criteria

  • Open deployment preview link OR navigate to branch ap-sm-delete-item.
  • Log in to account.
  • Select 'dinner' list.
  • Navigate to List page.
  • Select an item to delete - click the 'X' next to the checkbox.
  • Confirm that dialog box pops up to request confirmation of item deletion with the selected item's name.
  • When you click 'OK', the item should disappear from the List page.
  • Select another item to delete. When the confirmation dialog pops up, click 'Cancel'.
  • Ensure that the item does not disappear from the list.

Copy link

github-actions bot commented Mar 13, 2024

Visit the preview URL for this PR (updated for commit 334f1f3):

https://tcl-68-smart-shopping-list--pr33-ap-sm-delete-item-oljvk08l.web.app

(expires Sun, 24 Mar 2024 01:48:02 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 8cb5d089a99ba9972009993f4dd31796b0cbda84

@andiedoescode andiedoescode marked this pull request as ready for review March 13, 2024 16:47
DevinaG007
DevinaG007 previously approved these changes Mar 14, 2024
Copy link
Collaborator

@DevinaG007 DevinaG007 left a comment

Choose a reason for hiding this comment

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

Looks good! I do have one nit: I think the delete button could be made more accessible since we're using an X for the text of the button, and it might not be 100% clear. Check out this article on accessible buttons that I found. Otherwise, lovely work as always!

Dancing bear

etiry
etiry previously approved these changes Mar 14, 2024
Copy link
Collaborator

@etiry etiry left a comment

Choose a reason for hiding this comment

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

Good work--simple and straightforward solution!

src/api/firebase.js Show resolved Hide resolved
src/components/ListItem.jsx Show resolved Hide resolved
raaynaldo
raaynaldo previously approved these changes Mar 15, 2024
Copy link
Collaborator

@raaynaldo raaynaldo left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@andiedoescode andiedoescode dismissed stale reviews from raaynaldo, etiry, and DevinaG007 via 995d3eb March 17, 2024 01:41
@andiedoescode
Copy link
Collaborator Author

Looks good! I do have one nit: I think the delete button could be made more accessible since we're using an X for the text of the button, and it might not be 100% clear. Check out this article on accessible buttons that I found. Otherwise, lovely work as always!

Dancing bear Dancing bear

Very valid nit and likely good practice to incorporate the accessibility assessment throughout the whole process!

Copy link
Collaborator

@DevinaG007 DevinaG007 left a comment

Choose a reason for hiding this comment

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

Looks good!
fabulous gif tony stark

@andiedoescode andiedoescode merged commit 66cc3ee into main Mar 17, 2024
2 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
5 participants