Add Delete List Functionality #44

trushmi · 2024-04-01T20:57:40Z

Description

Currently, users can create lists on the home page, but there is no option to remove them if they are no longer needed. This PR aims to enhance the user experience by adding a delete option for lists on the home page.
When users click the delete button, they'll receive a confirmation message reminding them that deleting the list will also delete all items within it. This extra step helps prevent accidental deletions and gives users a chance to reconsider. With this feature, users will have more control over managing their lists and the opportunity to delete a list if they no longer need it.

Related Issue

Closes #17

Acceptance Criteria

On the home page, add a delete button to every list the user has on the home page.
If the user clicks delete, the system should show a message asking if the user is sure they want to delete the list and inform them that all items in the selected list will also be deleted. If the user confirm their choice, delete the list along with all its items.
Create a deleteList function in firebase.js to remove a list from the Firestore database.

Type of Changes

enhancement

Updates

Before

After

Note:
User can delete only their own list:

Testing Steps / QA Criteria

git pull
git checkout it-delete-single-list
npm ci
npm start

Navigate to the Home page.
Select a list and click on the delete button next to it.
Check if you see a message asking the user if they want to delete the selected list.
Select 'cancel' and check if the list is still present.
Repeat the previous steps and select 'yes'. Check if the list was deleted and if you see an alert on your screen with a message stating that the selected list was successfully deleted.
Try to delete a shared list with you. I have disabled deleting a list for people who have access to it but are not the owners
Navigate to Firebase and check if the list was deleted. If you see a list in the database, click on the name. You should see a message: "This document does not exist; it will not appear in queries or snapshots."

sillytsundere · 2024-04-01T22:37:48Z

Hey Iryna, when I went to review this it looks like the button doesn't have any functionality. When I click the delete buttons there is no confirmation message pop up nor do any of the lists get deleted.

…component

github-actions · 2024-04-02T05:19:40Z

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

https://tcl-73-smart-shopping-list--pr44-it-delete-single-lis-dyydgd2z.web.app

_{(expires Thu, 11 Apr 2024 16:43:31 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 99eed22fcca8bd9874e77f7b7f7d1eeb554a1666}

…e not the owner

rachelspencer

Hey @trushmi!

Awesome job making the new FireStore api to delete the list and items!

I realized that this only works for new lists, so we will all need to remember to clean up our FireStore before demos!

I left some comments in the code that relate to styling the delete button, so it matches how I have styled it on the List page.

Nice work!

rachelspencer · 2024-04-02T16:04:41Z

src/components/SingleList.jsx

 	return (
 		<div className="SingleList">
 			<li>
 				<button onClick={handleClick}>{name}</button>
+				<button aria-label={`Delete ${name}`} onClick={deleteSelectedList}>


If you put className="delete-btn" here, when this issue and my issue for the List styling is merged, the same styling I applied to the delete buttons on the List page will be applied.

I made a delete-btn global rule in the index.css, as I knew you were adding a delete button on the Home page, and that we would want the styling to be the same.

Hi @rachelspencer, I have removed the 'Delete' text and will also add the className="delete-btn" to maintain consistency in our styling!

rachelspencer · 2024-04-02T16:07:12Z

src/components/SingleList.jsx

 	return (
 		<div className="SingleList">
 			<li>
 				<button onClick={handleClick}>{name}</button>
+				<button aria-label={`Delete ${name}`} onClick={deleteSelectedList}>
+					<FontAwesomeIcon icon={faTrashCan} /> Delete


I also removed the Delete text in between the opening and closing tag.

Our aria label makes the icon accessible, so I removed the text, to make a cleaner UI, and to be closer to our wireframe designs.

If you do this also, our delete button styling will be consistent :-).

sillytsundere

Works great!! Thanks for getting this done Iryna! :3

jaguilar89

Looks good @trushmi! We'll have to remember to remove really old lists since this only works with lists that were created recently.

One issue I have with the list page in general is that right now there's no way to differentiate owned lists vs shared lists, you have to click into a list and see who the owner is.

Perhaps we could do something like add badges (like the purchase urgency badges) to shared lists that mark them as being shared, and at the same time, either disable the delete buttons or conditionally render them based on if the user is the owner or not.

@rachelspencer @sillytsundere What are your thoughts?

trushmi · 2024-04-03T18:40:46Z

Hey Iryna, when I went to review this it looks like the button doesn't have any functionality. When I click the delete buttons there is no confirmation message pop up nor do any of the lists get deleted.

Thanks for pointing this out @sillytsundere ! I fixed this issue!

trushmi · 2024-04-03T18:44:47Z

Looks good @trushmi! We'll have to remember to remove really old lists since this only works with lists that were created recently.

One issue I have with the list page in general is that right now there's no way to differentiate owned lists vs shared lists, you have to click into a list and see who the owner is.

Perhaps we could do something like add badges (like the purchase urgency badges) to shared lists that mark them as being shared, and at the same time, either disable the delete buttons or conditionally render them based on if the user is the owner or not.

@rachelspencer @sillytsundere What are your thoughts?

Hi @jaguilar89 Thank you for bringing this to my attention! It's a great observation and definitely worth investigating further!

polly89

Great job implementing this functionality. I left a few non-blocking comments.

polly89 · 2024-04-03T20:20:14Z

src/api/firebase.js

+	const userDoc = await getDoc(userRef);
+
+	if (listDoc.data().ownerID !== userDoc.data().uid) {
+		throw new Error('You are not the owner of this list.');


Great job! If you wanted to be even more explicit, you could add "You can't delete this list because you are not the owner."

It also brings up an interesting use-case. Perhaps in the future, you could give the user the option to hide a list if it's not theirs. It's not something I would tackle this week, as you are all busy meeting deadlines, but something interesting to consider if you choose to work on this once the cohort is done.

Thanks, @polly89! I will make the text more explicit and consider this use case!

polly89 · 2024-04-03T20:20:17Z

src/api/firebase.js

+		return { success: true, message: 'List successfully deleted' };
+	} catch (error) {
+		throw new Error(`Could not delete currunt list. An error: ${error}`);
+	}


nitpick - there's a small typo in the word "current".

Thanks! Fixed!

polly89 · 2024-04-03T20:22:41Z

src/index.jsx

@@ -2,7 +2,6 @@ import { StrictMode } from 'react';
 import { createRoot } from 'react-dom/client';
 import { Theme } from '@radix-ui/themes';
 import { App } from './App';
-import { Theme } from '@radix-ui/themes';



I actually deleted this line and pushed it to main earlier, as I had seen a comment regarding this on Slack.

polly89 · 2024-04-03T20:26:32Z

src/components/SingleList.jsx

+		if (
+			window.confirm(
+				`Are you sure you want to delete the ${name} list? This action cannot be undone and all items in this list will be deleted.`,
+			)


I know you all are using a library for the modals, but it would be great if we could change the "OK" button in this message in particular to "Delete" and make it red, since that's the industry standard.

Thank you for this recommendation, @polly89! I'll take the time to explore that further.

trushmi added the enhancement New feature or request label Apr 1, 2024

“Iryna added 2 commits April 1, 2024 22:10

add a delete button to the SingleList component

048105e

add delete single list functionality to firebase file and singleList …

c347406

…component

trushmi force-pushed the it-delete-single-list branch from 226d930 to c347406 Compare April 2, 2024 05:15

fix eslint error with Theme

a753c5a

disable deleting a list for people who have access to the list but ar…

982a9c8

…e not the owner

trushmi changed the title ~~add a delete button to the SingleList component~~ Add Delete List Functionality Apr 2, 2024

trushmi requested review from jaguilar89, sillytsundere and rachelspencer April 2, 2024 06:31

trushmi self-assigned this Apr 2, 2024

rachelspencer approved these changes Apr 2, 2024

View reviewed changes

remove text from the delete button

dfdf371

sillytsundere approved these changes Apr 2, 2024

View reviewed changes

jaguilar89 approved these changes Apr 3, 2024

View reviewed changes

polly89 approved these changes Apr 3, 2024

View reviewed changes

“Iryna and others added 2 commits April 3, 2024 14:25

update message to the user

dd981c0

Merge branch 'main' into it-delete-single-list

e96ab50

trushmi merged commit 22ee641 into main Apr 4, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Delete List Functionality #44

Add Delete List Functionality #44

trushmi commented Apr 1, 2024 •

edited

Loading

sillytsundere commented Apr 1, 2024

github-actions bot commented Apr 2, 2024 •

edited

Loading

rachelspencer left a comment

rachelspencer Apr 2, 2024

trushmi Apr 3, 2024

rachelspencer Apr 2, 2024

sillytsundere left a comment

jaguilar89 left a comment

trushmi commented Apr 3, 2024

trushmi commented Apr 3, 2024

polly89 left a comment

polly89 Apr 3, 2024

trushmi Apr 3, 2024

polly89 Apr 3, 2024

trushmi Apr 3, 2024

polly89 Apr 3, 2024

polly89 Apr 3, 2024

trushmi Apr 3, 2024

Add Delete List Functionality #44

Add Delete List Functionality #44

Conversation

trushmi commented Apr 1, 2024 • edited Loading

Description

Related Issue

Acceptance Criteria

Type of Changes

Updates

Before

After

Testing Steps / QA Criteria

sillytsundere commented Apr 1, 2024

github-actions bot commented Apr 2, 2024 • edited Loading

rachelspencer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sillytsundere left a comment

Choose a reason for hiding this comment

jaguilar89 left a comment

Choose a reason for hiding this comment

trushmi commented Apr 3, 2024

trushmi commented Apr 3, 2024

polly89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trushmi commented Apr 1, 2024 •

edited

Loading

github-actions bot commented Apr 2, 2024 •

edited

Loading