Add new item to the shopping list #17

ToobaJamal · 2023-04-09T15:46:59Z

Co-authored-by: shahx95 [email protected]

Description

This change allows users to add new items and an estimate of when they need to buy the items to the shopping list. The token is accessed from localStorage and passed to the addItem function so the users can add items to their shopping list when they submit the addItem form.

Related Issue

Closes #4

Acceptance Criteria

The Add Item view displays a form that allows them to enter the name of the item and select how soon they anticipate needing to buy it again. There should be 3 choices for this:
“Soon”, corresponding to 7 days
“Kind of soon”, corresponding to 14 days
“Not soon”, corresponding to 30 days
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 item either was or was not saved to the database.

Data-related tasks:

The console.log in the addItem function in src/api/firebase.js is replaced with a function that adds the new document to the Firestore database. That function will be imported from the firebase/firestore module.
The user’s soon/not soon/kind of soon choice is used to calculate nextPurchasedDate

Type of Changes

	Type
	🐛 Bug fix
✓	✨ New feature
	🔨 Refactoring
	💯 Add tests
	🔗 Update dependencies
	📜 Docs

Updates

Before

After

task4_after.mp4

Testing Steps / QA Criteria

If the user enters both the item name and the estimate of when they need to buy the item, Item has been added to the list will be displayed on the browser. If any of the input fields are missing, the user will see the Please enter valid inputs message on the screen.

This commit serves the purpose of creating a draft PR Co-authored-by: shahx95 <[email protected]>

github-actions · 2023-04-09T15:48:16Z

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

https://tcl-60-smart-shopping-list--pr17-sa-tj-add-item-shopp-8m7wl1bq.web.app

_{(expires Sat, 22 Apr 2023 07:50:27 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 93ba99965233232f14cf05d9d95de4adc510bd3b}

src/views/AddItem.jsx

shahx95 · 2023-04-10T18:39:27Z

src/views/AddItem.jsx

+
+ function handleSubmit(event) {
+ event.preventDefault();
+ if (formData.itemName !== '' && formData.daysUntilNextPurchase !== '') {


Instead of using if (formData.itemName !== '' && formData.daysUntilNextPurchase !== ''), we could simplify the condition by utilizing truthiness, like so: if (formData.itemName && formData.daysUntilNextPurchase). This approach considers any non-empty string input as valid for our purposes.

After the data enters the if statement but before it is sent to the database, we can format it to all lowercase. This will ensure consistency in the data and prevent any potential issues like duplicates arising from case sensitivity.

It is not explicitly mentioned in the scope of this problem, however. What do you think?

I think we have another issue for this.

Ah ok. That's cool! 😊

…ge state

src/views/AddItem.jsx

…-collab-lab/tcl-60-smart-shopping-list into sa-tj-add-item-shopping-list

shahx95 · 2023-04-12T14:14:19Z

As per your previous suggestion, I implemented the form clearance feature after submission. I also added retrieval of data from local storage. Kindly review the changes made accordingly.

shahx95 · 2023-04-12T14:15:18Z

If everything looks good, one of us can go ahead and remove all the console logs before we go ahead with the review and submission. @ToobaJamal

labrocadabro

Looks good! I do have a few comments, but nothing that would prevent merging.

If you move the confirmation message below the submit button, then you won't need to worry about the button shifting anymore. I think that's the more common position anyway.
I'm not a fan of error messages that automatically disappear. If the user misses it, they won't have any idea why their submission didn't go through & might even think that it did. This can lead to frustration.
For accessibility purposes, you might want to look into aria-live for the text that appears and disappears: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Live_Regions

shagunZ

Everything looks good to me!!

ToobaJamal · 2023-04-14T09:44:46Z

Looks good! I do have a few comments, but nothing that would prevent merging.

If you move the confirmation message below the submit button, then you won't need to worry about the button shifting anymore. I think that's the more common position anyway.

I'm not a fan of error messages that automatically disappear. If the user misses it, they won't have any idea why their submission didn't go through & might even think that it did. This can lead to frustration.

For accessibility purposes, you might want to look into aria-live for the text that appears and disappears: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Live_Regions

Thank you for adding these points, Laura. These are helpful :)

shahx95 · 2023-04-14T16:17:31Z

@labrocadabro thanks for the feedback.

If you move the confirmation message below the submit button, then you won't need to worry about the button shifting anymore. I think that's the more common position anyway.

I'm not a fan of error messages that automatically disappear. If the user misses it, they won't have any idea why their submission didn't go through & might even think that it did. This can lead to frustration.

I am also not entirely convinced by the current placement and user experience of the error message as it is currently. As mentioned by @ToobaJamal, we could potentially address this during the styling phase, so we may consider it a minor issue for now and revisit it later. @paramsiddharth, I would appreciate your opinion on this matter as well.

paramsiddharth · 2023-04-14T16:32:13Z

src/views/AddItem.jsx

+ try {
+ const convertedFormData = {
+ itemName: formData.itemName,
+ daysUntilNextPurchase: Number(formData.daysUntilNextPurchase),


Is it better to use Number(str) than parseInt(str)? There's no right answer, so I'll let you discover and decide on your own! 😃 Here's something you might want to check out: https://stackoverflow.com/questions/4090518/what-is-the-difference-between-parseint-and-number

paramsiddharth · 2023-04-14T16:33:03Z

src/views/AddItem.jsx

@@ -1,3 +1,104 @@
+import React from 'react';


A part of the suggestion for my next comment.

Suggested change

import React from 'react';

import React, { useState } from 'react';

paramsiddharth · 2023-04-14T16:34:40Z

src/views/AddItem.jsx

+ const [formData, setFormData] = React.useState({
+ itemName: '',
+ daysUntilNextPurchase: '',
+ });
+ const [submissionStatus, setSubmissionStatus] = React.useState('');
+ const [showMessage, setShowMessage] = React.useState(false);


If we're using the useState hook so many times, wouldn't it be a better idea to just import it instead of writing React.useState everytime? :) Think about it, it is always encouraged to do this in production code. If you're using a property more than 3 times and it is possible to import it selectively, the key is to do that.

Suggested change

const [formData, setFormData] = React.useState({

itemName: '',

daysUntilNextPurchase: '',

});

const [submissionStatus, setSubmissionStatus] = React.useState('');

const [showMessage, setShowMessage] = React.useState(false);

const [formData, setFormData] = useState({

itemName: '',

daysUntilNextPurchase: '',

});

const [submissionStatus, setSubmissionStatus] = useState('');

const [showMessage, setShowMessage] = useState(false);

I have made the requested changes :)

paramsiddharth · 2023-04-14T16:36:00Z

src/views/AddItem.jsx

+ function handleChange(event) {
+ const { name, value } = event.target;
+
+ setFormData((prevFormData) => {
+ return {
+ ...prevFormData,
+ [name]: value,
+ };
+ });
+ }


I admire how you two wrote a generic function that would work for any field. 🌷 Impressive!

Edit: This one is just an appreciation comment. :) No change is needed.

paramsiddharth

A few changes are needed, and then you'll be good to go! :) Excellent and quality work on the PR, Shah and Tooba!

RajGM · 2023-04-14T18:28:11Z

@shahx95 absorbs office hour insight and PR feedback very quickly.
Great work @shahx95 and @ToobaJamal

RajGM

LGTM

labrocadabro · 2023-04-15T03:59:44Z

@labrocadabro thanks for the feedback.

If you move the confirmation message below the submit button, then you won't need to worry about the button shifting anymore. I think that's the more common position anyway.

I'm not a fan of error messages that automatically disappear. If the user misses it, they won't have any idea why their submission didn't go through & might even think that it did. This can lead to frustration.

I am also not entirely convinced by the current placement and user experience of the error message as it is currently. As mentioned by @ToobaJamal, we could potentially address this during the styling phase, so we may consider it a minor issue for now and revisit it later. @paramsiddharth, I would appreciate your opinion on this matter as well.

@shahx95 This is why I asked on slack if we should be addressing UI/UX concerns as we go. I'm unclear on whether such issues are scope creep, or if they're supposed to be addressed but are up to the discretion of the authors and therefore difficult to put in the acceptance criteria.

Would appreciate it if @paramsiddharth, @RajGM, or @iamkrati22 could weigh in.

paramsiddharth

Excellent work, Tooba and Shah! <3 Looks good to me!

paramsiddharth

Excellent job, Tooba and Shah! <3 I'm impressed. Let's merge it now!

Add task comment

dd15537

This commit serves the purpose of creating a draft PR Co-authored-by: shahx95 <[email protected]>

ToobaJamal changed the title ~~Add task comment~~ Add new item Apr 10, 2023

shahx95 and others added 4 commits April 10, 2023 20:47

Add form and form handle functions

a1fd562

Add radio buttons

8e6c1a3

Implement handle change function

21d3174

Add handle submit

e7c2e4c

shahx95 reviewed Apr 10, 2023

View reviewed changes

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

shahx95 reviewed Apr 10, 2023

View reviewed changes

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

shahx95 reviewed Apr 10, 2023

View reviewed changes

ToobaJamal and others added 2 commits April 11, 2023 14:23

Refactor: Fix element IDs, relocate semantic labels and add showMessa…

9447c8f

…ge state

Refactor logical rendering to use ternary operator with null

9f727ef

shahx95 reviewed Apr 11, 2023

View reviewed changes

src/views/AddItem.jsx Show resolved Hide resolved

ToobaJamal and others added 6 commits April 11, 2023 20:45

Add addDoc function

1c3f17e

Merge branch 'sa-tj-add-item-shopping-list' of https://github.com/the…

afee23b

…-collab-lab/tcl-60-smart-shopping-list into sa-tj-add-item-shopping-list

Convert and pass formData to addItem

a9e9aed

Refactor AddItem.jsx

6793be6

Retrieve token from local storage

2c4e1c9

Clear form after submission

653befd

ToobaJamal changed the title ~~Add new item~~ Add new item to the shopping list Apr 12, 2023

Remove unnecessary console logs

b785abf

shahx95 requested review from labrocadabro, RajGM, paramsiddharth and shagunZ April 12, 2023 15:42

ToobaJamal marked this pull request as ready for review April 12, 2023 15:43

labrocadabro approved these changes Apr 13, 2023

View reviewed changes

shagunZ approved these changes Apr 13, 2023

View reviewed changes

paramsiddharth requested a review from iamkrati22 April 14, 2023 16:19

paramsiddharth reviewed Apr 14, 2023

View reviewed changes

paramsiddharth requested changes Apr 14, 2023

View reviewed changes

RajGM approved these changes Apr 14, 2023

View reviewed changes

Replace react.useState with useState from import

a1ac77b

paramsiddharth approved these changes Apr 15, 2023

View reviewed changes

Add setSubmissionStatus for error

f9a150c

paramsiddharth approved these changes Apr 15, 2023

View reviewed changes

paramsiddharth merged commit f9ef46d into main Apr 15, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new item to the shopping list #17

Add new item to the shopping list #17

ToobaJamal commented Apr 9, 2023 •

edited

github-actions bot commented Apr 9, 2023 •

edited

shahx95 Apr 10, 2023

shahx95 Apr 10, 2023

ToobaJamal Apr 10, 2023

shahx95 Apr 10, 2023

shahx95 commented Apr 12, 2023

shahx95 commented Apr 12, 2023

labrocadabro left a comment •

edited

shagunZ left a comment

ToobaJamal commented Apr 14, 2023

shahx95 commented Apr 14, 2023 •

edited

paramsiddharth Apr 14, 2023

paramsiddharth Apr 14, 2023

paramsiddharth Apr 14, 2023

ToobaJamal Apr 15, 2023

paramsiddharth Apr 14, 2023

paramsiddharth Apr 14, 2023

paramsiddharth left a comment

RajGM commented Apr 14, 2023 •

edited

RajGM left a comment

labrocadabro commented Apr 15, 2023

paramsiddharth left a comment

paramsiddharth left a comment

	import React from 'react';
	import React, { useState } from 'react';

Add new item to the shopping list #17

Add new item to the shopping list #17

Conversation

ToobaJamal commented Apr 9, 2023 • edited

Description

Related Issue

Acceptance Criteria

Data-related tasks:

Type of Changes

Updates

Before

After

Testing Steps / QA Criteria

github-actions bot commented Apr 9, 2023 • edited

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

shahx95 commented Apr 12, 2023

shahx95 commented Apr 12, 2023

labrocadabro left a comment • edited

Choose a reason for hiding this comment

shagunZ left a comment

Choose a reason for hiding this comment

ToobaJamal commented Apr 14, 2023

shahx95 commented Apr 14, 2023 • edited

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

paramsiddharth left a comment

Choose a reason for hiding this comment

RajGM commented Apr 14, 2023 • edited

RajGM left a comment

Choose a reason for hiding this comment

labrocadabro commented Apr 15, 2023

paramsiddharth left a comment

Choose a reason for hiding this comment

paramsiddharth left a comment

Choose a reason for hiding this comment

ToobaJamal commented Apr 9, 2023 •

edited

github-actions bot commented Apr 9, 2023 •

edited

labrocadabro left a comment •

edited

shahx95 commented Apr 14, 2023 •

edited

RajGM commented Apr 14, 2023 •

edited