Issue#4: Set up new shopping list #23

zahrafalak · 2024-08-23T03:56:56Z

Description

A shopping list is a set of items associated with the owner's unique id and the name of the shopping list. Users need to be able to create new shopping lists. To do this, the Home view should present them with a form that allows them to enter the name of their list. Additionally, we need to do a few things to create that list:

Save the list path to localStorage
Save the list to our database using the createList function in firebase.js
Show the user the list view

Related Issue

closes #4

Acceptance Criteria

UI-related tasks:

The Home view displays a form that allows users to enter the name of a shopping list and then create a list with that name.
The input that accepts the name of the item has a semantic label element associated with it
The user can submit this form with both the mouse and the Enter key
When the user submits the form, they see a message indicating that the list either was or was not created and saved to the database.

Data-related tasks:

Clicking the button uses the createList function in src/api/firebase.js to create a new shopping list for the user.
The shopping list path is stored in local storage using the setListPath function.
Once the list has been created and saved, the user is redirected to the List view.

Type of Changes

new feature

Updates

Before

After

Issue.4_recording.mov

Testing Steps / QA Criteria

Pull the branch locally using git pull origin fz-ma-setup-new-shopping-list in your terminal.
Make sure you are signed into the app.
On the Home Page, you will see a form with a field to enter the name of a new shopping list. Enter the name of your new list.
Clicking "Create List" will create the new list in the database, and will show an alert if the list was successfully created.
The user is them redirected to the list view.
Open up the console and check out local storage. The new list's path will show up there after successful creation.

github-actions · 2024-08-23T03:57:51Z

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

https://tcl-77-smart-shopping-list--pr23-fz-ma-setup-new-shop-vrpb6qzg.web.app

_{(expires Sun, 01 Sep 2024 18:18:16 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72}

RossaMania · 2024-08-23T18:27:13Z

I like how the handleSubmit uses a try-catch block! I like how there's an alert for successful list creation, rather than a console log! I like how the error is logged with that catch block!

Have you guys thought about implementing React Hot Toast as well?

zahrafalak · 2024-08-23T19:09:40Z

I like how the handleSubmit uses a try-catch block! I like how there's an alert for successful list creation, rather than a console log! I like how the error is logged with that catch block!

Have you guys thought about implementing React Hot Toast as well?

Thanks for the kind words Ross! Yes, we did look into that as well, and also noticed that y'all are using it too for the list view. It also visually looks better than an alert. We can definitely add that in to keep the app consistent and seamless.

src/views/Home.jsx

bbland1 · 2024-08-23T20:43:22Z

src/views/Home.jsx

+		e.preventDefault();
+
+		try {
+			await createList(userId, userEmail, inputValue);


suggestion: I know Ross also mentioned using react-hot-toast for the alert and with your suggestion on ours about clearing the form when done I looked more in how to use the toast to handle the success and the error. so if yall want to also use it I think it would look something like this for yalls call

Suggested change

await createList(userId, userEmail, inputValue);

await toast.promise(

createList(userId, userEmail, inputValue),

{

pending: , // a message about adding new list

success: () => {

const path = `${user.uid}/${inputValue}`;

setListPath(path);

setInputValue('');

navigate('/list');

return 'Success: Your new list is created!';

},

error: () => {

return 'Failed to create the list. Please try again.';

},

},

{

style: {

minWidth: '250px',

},

},

);

Thanks for the suggested code Brianna! We tried a tad simpler approach for now (as shown below), and can definitely refactor if needed in the future.

src/views/Home.jsx

bbland1 · 2024-08-23T20:53:17Z

Left some praise and and comments! Y'all did awesome and it is so cool to see the changes and app progressing! 😎

tannaurus · 2024-08-24T17:55:04Z

src/App.jsx

+							<Home
+								data={lists}
+								setListPath={setListPath}
+								userId={userId}


Looks like we're giving props to Home that aren't being used 👀 maybe we could use these instead of calling useAuth inside of Home?

Oh, thanks for catching that! We were trying 2 different approaches, one with passing props, and other with calling useAuth instead of Home (mainly to keep the component self contained), and likely forgot to remove the props in the process.
But I'm thinking it might be more beneficial to go the props route in this case, since Home already relies on other data passed on from the App.jsx anyway, and we can avoid redundant calls.

tannaurus · 2024-08-24T18:11:30Z

src/views/Home.jsx

+	const userId = user?.uid;
+	const userEmail = user?.email;
+
+	const [inputValue, setInputValue] = useState('');


Something that I like to see when building React apps is leaning into the idea of composing components. With the introduction of this feature, Home now has two very distinct roles:

display the lists that the user can access

create new lists

While this is perfectly functional and a great way to "get something working", when implementing production-ready React you'll often pair changes like these with unit tests written with libraries like jest and react-testing-library. Breaking down components to "own" these individual roles can be an excellent way to easily achieve either good test coverage, or at least a stronger insight into what does and doesn't have test coverage. If a component has 1 role, it can be easy to spot at a glance what the test coverage looks like for that role. When a component starts wearing more hats, things can start to slip through the cracks ☔

While writing unit tests isn't in the scope of this project, learning how to write code in ways that make unit testing easier is a skillset that's really good to start exercising; the senior devs at your next role will praise you for it 😄 As someone who spends a ton of my time reviewing code, having elements of code broken down into these "roles" that can then be composed together into a parent component like Home lets the me (the reviewer, any reviewer, and especially post-review "I've got to figure out what this code is doing so I can make changes to it" teammates) quickly grasp what is and isn't happening in a specific diff, and can go a long way towards the health of a codebase 🚀

and just to clarify, I don't think this needs to lead to changes in this PR, just something to keep in mind 😄

We can absolutely cover testing if it's something the group is interested in, we should chat at our next meeting!

tannaurus · 2024-08-24T19:13:26Z

src/views/Home.jsx

+
+	const handleSubmit = async (e) => {
+		e.preventDefault();
+


We might want to trim the inputValue and ensure it still has a length > 0 before creating the list. We currently allow for a single space character to be used as the list name, which results in some funky UI behavior. Generally it's pretty common for users to accidentally prepend or append a space to their inputs accidentally; in cases like these I like to help them out by trimming the input 😄

That is a great callout for form validations! 😅 Definitely don't wan't garbage data going into the database.

tannaurus · 2024-08-24T19:34:02Z

src/views/Home.jsx

+			const path = `${user.uid}/${inputValue}`;
+			setListPath(path);
+			setInputValue('');
+			alert('Success: Your New List is Created!');


Quoting my comment from PR #22

Something that is nearly universally enforced by a linter, but doesn't appear to be here, is consistency between single and double quotes. I notice some inconsistencies here, which isn't a big deal, but it's usually nice to see picking between one or the other (I'm team double quotes personally) and sticking to it

Great observation! I try using single quotes in everything, but some of them get changed to doubles quotes automatically when I save the file. I believe it might be the code formatter I'm using (Prettier) that's causing that. 🤔

Ah you're correct! We had a prettier rule enforcing the inconsistency 🤦 I've fixed that here!

alex-andria

Great job Falak and Maha for knocking this out! Love the suggestion and discourse happening in this PR.

+1 on suggesting having a uniform notification UI throughout the app whether that's react-hot-toast or something else. Something to think about maybe later on is that the notification still alerts with a successful message even if the list name is a duplicate list name. Might be a good use case to address later to alert with a "Uh-oh that shopping list name already exists!" potentially.

zahrafalak added 2 commits August 22, 2024 17:16

Add new list and render it on Home page

85fe32c

Add alerts for success and failure. Redirect to list view on success.

1f7c03f

zahrafalak requested review from adidalal, tannaurus, alex-andria, RossaMania and bbland1 August 23, 2024 03:56

zahrafalak assigned eternalmaha and zahrafalak Aug 23, 2024

bbland1 reviewed Aug 23, 2024

View reviewed changes

src/views/Home.jsx Outdated Show resolved Hide resolved

bbland1 reviewed Aug 23, 2024

View reviewed changes

src/views/Home.jsx Outdated Show resolved Hide resolved

bbland1 reviewed Aug 23, 2024

View reviewed changes

src/views/Home.jsx Outdated Show resolved Hide resolved

tannaurus reviewed Aug 24, 2024

View reviewed changes

tannaurus mentioned this pull request Aug 24, 2024

Feat: Allowing user to add items to shopping list #22

Merged

6 tasks

tannaurus reviewed Aug 24, 2024

View reviewed changes

alex-andria approved these changes Aug 25, 2024

View reviewed changes

Add form validation and react-hot-toast notifications on submit.

1479c6b

tannaurus mentioned this pull request Aug 25, 2024

Prettier using double quotes #25

Merged

zahrafalak and others added 3 commits August 25, 2024 10:54

Move the code for creating new list into it's own component

be22175

Resolve conflict with package lock

f63da1f

Merge branch 'main' into fz-ma-setup-new-shopping-list

51d58fb

zahrafalak merged commit fc92917 into main Aug 25, 2024
2 checks passed

zahrafalak deleted the fz-ma-setup-new-shopping-list branch August 25, 2024 18:25

tannaurus mentioned this pull request Aug 31, 2024

#6, feat: Add form that allows user to invite another user to list! #28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue#4: Set up new shopping list #23

Issue#4: Set up new shopping list #23

zahrafalak commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

RossaMania commented Aug 23, 2024

zahrafalak commented Aug 23, 2024

bbland1 Aug 23, 2024

zahrafalak Aug 25, 2024

bbland1 commented Aug 23, 2024

tannaurus Aug 24, 2024

zahrafalak Aug 25, 2024

tannaurus Aug 24, 2024 •

edited

Loading

adidalal Aug 24, 2024

tannaurus Aug 24, 2024 •

edited

Loading

zahrafalak Aug 25, 2024

tannaurus Aug 24, 2024

zahrafalak Aug 25, 2024

tannaurus Aug 25, 2024

alex-andria left a comment

-			await createList(userId, userEmail, inputValue);
+			await toast.promise(
+				createList(userId, userEmail, inputValue),
+				{
+					pending: , // a message about adding new list
+					success: () => {
+						const path = `${user.uid}/${inputValue}`;
+                        setListPath(path);
+                        setInputValue('');
+                        navigate('/list');
+                        return 'Success: Your new list is created!';
+					},
+					error: () => {
+						return 'Failed to create the list. Please try again.';
+					},
+				},
+				{
+					style: {
+						minWidth: '250px',
+					},
+				},
+			);

Issue#4: Set up new shopping list #23

Issue#4: Set up new shopping list #23

Conversation

zahrafalak commented Aug 23, 2024 • edited Loading

Description

Related Issue

Acceptance Criteria

Type of Changes

Updates

Before

After

Testing Steps / QA Criteria

github-actions bot commented Aug 23, 2024 • edited Loading

RossaMania commented Aug 23, 2024

zahrafalak commented Aug 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbland1 commented Aug 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannaurus Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannaurus Aug 24, 2024 • edited Loading

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

alex-andria left a comment

Choose a reason for hiding this comment

zahrafalak commented Aug 23, 2024 •

edited

Loading

github-actions bot commented Aug 23, 2024 •

edited

Loading

tannaurus Aug 24, 2024 •

edited

Loading

tannaurus Aug 24, 2024 •

edited

Loading