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

Dc el delete items #28

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Dc el delete items #28

wants to merge 4 commits into from

Conversation

eva-lng
Copy link
Collaborator

@eva-lng eva-lng commented Sep 16, 2024

Type
New Feature
Delete an item from the list

Description

A delete button is rendered next to each item in a list. When clicked, the item is deleted from the database.

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

Updated UI with rendered delete buttons and pop up window
Screenshot 2024-09-16 at 22 06 04

Testing Steps / QA Criteria

  1. npm start
  2. navigate to List view
  3. click on the delete button next to one of the items
  4. confirm the pop up message

Copy link

github-actions bot commented Sep 16, 2024

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

https://tcl-78-smart-shopping-list--pr28-dc-el-delete-items-817nxxwp.web.app

(expires Thu, 26 Sep 2024 16:52:41 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c781903507c1507075d7a974036959ddeec29c0a

Copy link
Contributor

@tataw-cl tataw-cl left a comment

Choose a reason for hiding this comment

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

Excellent work done guys. It works as expected 👌🏾. And whooooo, y'all's speed was something else 😅🔥

Copy link
Collaborator

@Amaka202 Amaka202 left a comment

Choose a reason for hiding this comment

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

Well done team!, your feature works mostly.💥
but when I have an internet issue and my delete request could not be completed, I don't see an error message saying why. That will def reduce the users' experience on our app.

I left a suggestion on how you can go about it😊. Let me know if you have any questions!

cc: @didemydn @eva-lng

Comment on lines 233 to 235
} catch (error) {
console.log(error);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can throw the error here and catch it in the component where you are calling deleteItem for appropriate handling

Comment on lines 34 to 36
} catch (error) {
console.error('Failed to delete the item', error);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

you already have a system of error management from your past issues, you can use it here to show the user an error message. it is always good for a developer to show a good error message to a user as to why an action could not be completed.

@GrannyYetta
Copy link
Collaborator

Well done! The item deletion works well for me :) Congrats on the speediest issue yet!

Comment on lines +36 to +38
} catch (error) {
console.error(error.message, error);
setErrorMsg('Failed to delete the item. Please try again!');
Copy link
Collaborator

@Amaka202 Amaka202 Sep 19, 2024

Choose a reason for hiding this comment

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

great! well done team!!😊💥

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