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

RossaMania · 2024-08-28T22:07:26Z

For an example of how to fill this template out, see this Pull Request.

Description

We want to allow users to invite others to existing shopping lists so they can manage them with friends or family. We created a form in the ManageList view that allows the user to enter an email to invite an existing user to a list. It's in addition to the form that allows them to add items to that list. The user can submit this form with both the mouse and the Enter key. If the other user exists, the user is alerted that the list was shared with a React Hot Toast notification. If the other user does not exist, the list will not be shared and user will be notified!

This uses event listeners, and the shareList function from the firebase.ts!

We learned that it is important to keep it simple. While it may feel like we're reinventing the wheel even though we went from JavaScript to TypeScript in week 3, this is still a CRUD app. It was so cool to go through the different files and see how all the different arguments and props were passed!

Related Issue

closes #6

Acceptance Criteria

The ManageList view shows a form that allows the user to enter an email to invite an existing user to a list, in addition to the
form that allows them to add items to that list.
The input that accepts the email has a semantic label element associated with it
The user can submit this form with both the mouse and the Enter key
If the other user exists, the user is alerted that the list was shared
If the other user does not exist, the user is shown an error message that explains the problem

Type of Changes

enhancement

Updates

Before

After

Testing Steps / QA Criteria

Sign in to app.
Pick a list from the Home page.
Click on "Manage List"
Enter email address in the text field to the left of the "Invite User" button.
Click on "Invite User" button or press Enter.
Wait for the notification! The notification will either be green and show the email address and the message that the user was successfully invited to your list, or it will be red, show the email address, and the message that the user was not added to list!

…n exisiting user to a list. Use the shareList firebase func to add existing user to list. Co-authored-by: Maha Ahmed <[email protected]>

github-actions · 2024-08-28T22:09:01Z

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

https://tcl-77-smart-shopping-list--pr28-rc-ma-share-shopping-aq7r2o1j.web.app

_{(expires Sun, 08 Sep 2024 00:54:44 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72}

RossaMania · 2024-08-28T22:15:52Z

It was great working with you this week Maha!

I know we are not supposed to worry about styling, and I love that the acceptance criteria gave us some freedom! Do you think it would be a good idea to add something like a horizontal row (hr tag) between the two forms where that arrow is pointing?

How do you feel about placeholder text? What do you think about putting in "Enter e-mail address. . ." in the text field?

RossaMania · 2024-08-28T22:20:29Z

I'm so glad we got some more practice with React Hot Toast this week!

What do you think about changing the messages? Something like Successfully invited ${emailName} to your list! for success? Something like Oops! Failed to invite ${emailName} to your list. Please try again! for error?

The important parts we got done are:
A. The toasts are working!
B. The variable shows up in string interpolation.

src/views/ManageList.tsx

bbland1 · 2024-08-29T03:09:13Z

src/views/ManageList.tsx

+							Invite User
+						</button>
+					</form>
+					<Toaster />


note: I don't think there needs to be 2 <Toaster /> elements in this. It should only need one.

So cool! Maha and I were wondering that! We figured one form === one toaster element.

Yeah! We really were curious about that. I thought it was a per form as well. After looking into it some more, its good practice to "import { Toaster } from 'react-hot-toast' " into the root of the application , probably in our App.tsx file. By doing that, we will only need to " import { toast } from 'react-hot-toast' " into any of our needed files, but we wont need to place any elements since all our app components will have access to it :)

bbland1 · 2024-08-29T03:10:36Z

src/views/ManageList.tsx

+		console.log("Button clicked! Inviting user!");
+		e.preventDefault();
+
+		const trimmedEmailName = validateTrimmedString(emailName);


praise: I love seeing this util function validateTrimmedString being used again! How cool!

I do too! Except I don't think it needs to be 😄 because the input element has the email type, it isn't possible to get this component into a state where email has any whitespace, the input trims it for you already:

https://html.spec.whatwg.org/multipage/input.html#email-state-(type=email)

and for what it's worth, I didn't know this either 😄 I knew email wouldn't let an empty string through (the benefit of using email is that you don't have to worry about figuring out if the string is an email, which can't be an empty string). But I discovered through testing that it was also stripping the white space, which is very good to know!

bbland1 · 2024-08-29T03:20:52Z

src/views/ManageList.tsx

+
+		try {
+			await toast.promise(shareList(listPath, currentUser, trimmedEmailName), {
+				loading: "sharing list with existing user",


I'm so glad we got some more practice with React Hot Toast this week!

What do you think about changing the messages? Something like Successfully invited ${emailName} to your list! for success? Something like Oops! Failed to invite ${emailName} to your list. Please try again! for error?

The important parts we got done are:
A. The toasts are working!
B. The variable shows up in string interpolation.

I think this comment was more for Maha but I really like changing the messages to be a bit more descriptive! It would allow another way for the user to see like "who did i just invite?". 😅

Nothing wrong with answering questions! We can all work together. 😄

Yes! I like that idea too. I was playing around in the useAuth.tsx file, and I noticed we have access to user.name and can invoke the list owner/user's name into our notifications, but it would be so cool if we could insert the invited user's name instead of their email !!!!

bbland1 · 2024-08-29T03:28:22Z

src/views/ManageList.tsx

@@ -20,10 +22,13 @@ const purchaseTimelines = {
 };

 export function ManageList({ listPath }: Props) {
+	const { user: currentUser } = useAuth();


question: Was there a reason to call useAuth() here? I know the user is needed on line 110 and I'm just curious if it was considered adding the user prop to the <ManageList /> in App.tsx and add it to the props in the call here.

It was not! That's all we could think to do since currentUser had to come from somewhere.

src/views/ManageList.tsx

bbland1 · 2024-08-29T03:57:34Z

I'm not really sure where to put this so I'm just throwing it in a PR comment over all. I noticed when I typed in the email I am signed up with and an email of a not currently existing in the DB user I got a successful message both times.

Reading over the acceptance criteria

[ ] If the other user does not exist, the user is shown an error message that explains the problem

I think it should get an error at least if an email is passed of a user that does not currently exist within the application.

I'm not sure if it should send some type of error if the user tries to share the list with themselves acceptance-wise but it may be nice to think about!

Screen.Recording.2024-08-28.at.11.41.04.PM.mov

eternalmaha · 2024-08-29T04:50:58Z

Wow! I love how we noticed that at the exact same time :) "Async" hahah. You're absolutely right!! I just realized that it does in fact send a "successful notification" regardless of whether the user exists or not (any email was working). After looking at the shareList function that checks whether the invited user exists before sharing the list, the condition was written to "exit" the function early through a return statement if the "invited user" didn't exist. I learned that JavaScript considers a function "successful" if it returns a value or resolves itself without returning an error even if the function never completes. I believe then, the toast promise's error block didn't register the function exiting early as an "error". So interesting! Below I've screenshotted the before and after to fix the issue. Thanks for your detailed review @bbland1 ! What do you think @RossaMania ?

RossaMania · 2024-08-29T17:17:41Z

Great catches you guys! It's not letting me reply to comments directly! Yes. I agree that throwing an actual Error object with a message would help! I feel like the message should say something like "Unable to share list". A message like "Recipient doesn't exist" could cause security concerns. It could be seen as it has just confirmed to somebody that that is not valid user information.

bbland1 · 2024-08-29T17:39:11Z

That is awesome @eternalmaha! I saw it and I was like ooo I'm not sure I can describe it well so chose to take the screen recording lol 😅!

I also think getting the message clear so not to cause concerns is important @RossaMania!

eternalmaha · 2024-08-30T04:44:28Z

@RossaMania I think editing our wording in the notifications sent to the user to be more clear is a great suggestion! I think including that the user is "unable to share list" as you mentioned is important. I think it is also a good idea to include why the list can't be shared so the user is able to troubleshoot, that doesn't cause them unecessary alarm. Perhaps "Unable to share list. Verify user has an active account !" ?

I'm also down to make some basic styling changes to the appearance as well when we meet tomorrow to review and finalize all the suggested changes :)

…th every email entered

…ications across entire app and removed unecessary Toaster elements in child components

…ailed email invite

RossaMania · 2024-08-30T19:44:36Z

So cool! @eternalmaha just got done meeting!

We looked at everybody's suggestions so far!
We corrected spelling errors.
We changed the wording of toast messages in ManageList components!
Imported and rendered Toaster component at app level!
Added placeholder text for that email input, and added some spacing between forms with a hr element.
Removed all redundant Toaster component renders!

bbland1 · 2024-08-31T03:28:32Z

src/views/ManageList.tsx

+					return `Successfully invited ${emailName} to your list!`;
+				},
+				error: () => {
+					return `Oops! Failed to invite ${emailName} to your list. Please verify correct email!`;


praise: I really like this update to have the email name in the messages!

zahrafalak · 2024-08-31T05:30:10Z

First off, I want to say that you’ve done a fantastic job with the ManageList component! The way you've structured the core functionality is clear, and your use of react-hot-toast for user feedback adds a nice, polished touch to the user experience. Also, the validation checks you’ve implemented are spot-on, ensuring that only valid data gets processed. 🚀

That said, I have a suggestion that could make the code even more maintainable, and it's one of the things @tannaurus called out for in our code as well, which I think is a great thing to be mindful of when building React apps: 😄

To further enhance the structure, consider breaking the ManageList component down into smaller, focused components like ItemForm, InviteForm, and PurchaseTimelineSelector. This would not only keep the code more organized and easier to manage but would also make these components more reusable across the app, saving time and effort in future development. By separating the item addition and user invitation logic into distinct components, we can improve the overall clarity of the code, making it much easier to test and debug. Additionally, this approach would significantly boost the readability of the code and lay a strong foundation for any future enhancements or updates, ensuring that the code remains clean and maintainable as the project grows.

Again, excellent work on what you’ve built. The functionality is well-implemented, and the comment above is just a suggestion at this point, and can be implemented as a separate issue on it's own in upcoming future.

tannaurus · 2024-08-31T17:10:34Z

src/views/ManageList.tsx

+		}
+
+		if (currentUser === null) {
+			toast.error(`You are not logged in! Cannot invite.`);


I think it would be best to not render this component at all if the user is not signed in instead of notifying the user that they cannot do this operation after they have already attempted to do so

Yes please! We were getting an odd message (that I can't remember right now), and that null-check solved the issue.
BUT, thinking about it now, we could just do a ternary operator like,

{currentUser && //code here}

right?

Edited to add: I'm also going to try it out here in a bit!

Yup, that's correct! @bbland1 has been working to make this part of the codebase much more straight forward, so this shouldn't be an issue for much longer 😄 https://github.com/the-collab-lab/tcl-77-smart-shopping-list/pull/27/files

Perfect! Just make it as clean as possible to save on extra refactoring and feelings later!

RossaMania · 2024-08-31T17:13:16Z

Yes please! @zahrafalak I agree wholeheartedly! At this point, I feel like our app is getting to a nice size, and I was thinking about bringing up refactoring some elements into separate components this week.

What nested folders should components be in, in the components folder?
Are we going to do the arrow functional component export, or continue with the "export function" version?
Should we wait for the auth routes middleware to be done?
I can't think of the other questions I was thinking of, the other day.

tannaurus · 2024-08-31T17:21:13Z

src/views/ManageList.tsx

+
+		const trimmedEmailName = validateTrimmedString(emailName);
+
+		if (!trimmedEmailName) {


Similar to my note above, we can remove this check: it's not possible to hit it.

tannaurus · 2024-08-31T17:25:14Z

src/views/ManageList.tsx

@@ -76,6 +85,44 @@ export function ManageList({ listPath }: Props) {
 		}
 	};

+	const handleInvite = async (


Could all of this new logic be its own component that is rendered here instead? There's a problem developing in this file where 1. the component is doing too many things (re: this other comment I left on PR #23) and 2. now there's a function in this component called handleSubmit along side two different forms, making it hard to understand what that function is doing unless you read through its definition. This can be solved quickly by pushing this logic into its component and rendering it here instead 😄

Yes please! I have an idea of what I want to do! I can work on this!

Here are some screenshots of the component for your viewing pleasure!

WooooHoooo!!!!!

…orm submission and uses the `shareList` function from the Firebase API to add the existing user to the list. This feature enhances collaboration and sharing within the application.

…put type email. Corrected aria-label for handleInvite submit button

…is logged in

eternalmaha · 2024-09-01T00:37:25Z

Thank you everyone for the great input and conversation! My recent last commit:
-I had refactored the form submission to only render if currentUser is logged in. Though after some thought, I realized it is wise to leave the form submission displaying to all users (logged in or not) as it is a crucial feature of the application.

Instead I suggest it we implement a strict type assertion in the shareList functions' currentUser so shareList only expects a User, and never a null.

Since currentUser could be User | null, we can set currentUser as User after importing the User object from firebase. This way, TypeStrict only expects a logged in user. I tested it, and it worked. Next thing would be to add an alert to the user if they try to share a list, and are not logged in.

The other option would be to do a conditional check before the shareList function of user being logged in or not. Feel free to check any gaps in my logic! If everyone is on board, I'll go ahead and commit the change.

user.is.logged.in.to.share.list.mov

…e assertion of User for shareList function to only allow user to share list if logged in

feat: Add form that allows user to enter an email address to invite a…

32fb5bd

…n exisiting user to a list. Use the shareList firebase func to add existing user to list. Co-authored-by: Maha Ahmed <[email protected]>

RossaMania assigned RossaMania and eternalmaha Aug 28, 2024

RossaMania requested a review from tannaurus August 28, 2024 22:07

RossaMania changed the title ~~feat: Add form that allows user to invite another user to list!~~ #6, feat: Add form that allows user to invite another user to list! Aug 28, 2024

RossaMania requested review from alex-andria, bbland1 and zahrafalak August 28, 2024 22:08

bbland1 reviewed Aug 29, 2024

View reviewed changes

src/views/ManageList.tsx Outdated Show resolved Hide resolved

bbland1 reviewed Aug 29, 2024

View reviewed changes

eternalmaha closed this Aug 29, 2024

bbland1 reviewed Aug 29, 2024

View reviewed changes

eternalmaha reopened this Aug 29, 2024

bbland1 reviewed Aug 29, 2024

View reviewed changes

src/views/ManageList.tsx Show resolved Hide resolved

eternalmaha added 4 commits August 30, 2024 00:58

fixed error of toast notification always showing succesful promise wi…

83deee5

…th every email entered

refactored Toast object at App parent level to access hot toast notif…

05baa77

…ications across entire app and removed unecessary Toaster elements in child components

updated toast notification text and implemented a correct alert for f…

eceeec7

…ailed email invite

removed redundant Toaster elements throughout app

779cdbb

bbland1 reviewed Aug 31, 2024

View reviewed changes

bbland1 approved these changes Aug 31, 2024

View reviewed changes

tannaurus reviewed Aug 31, 2024

View reviewed changes

RossaMania and others added 3 commits August 31, 2024 16:09

refactor: The ShareListForm component is introduced to handle the f…

8d96ad3

…orm submission and uses the `shareList` function from the Firebase API to add the existing user to the list. This feature enhances collaboration and sharing within the application.

removed trimmedEmailName utility function as its not needed due to in…

391e36e

…put type email. Corrected aria-label for handleInvite submit button

refactored form submission to conditionally render form only if user …

5647082

…is logged in

eternalmaha added 2 commits August 31, 2024 20:48

removed the conditional render of form submission and implemented typ…

925581b

…e assertion of User for shareList function to only allow user to share list if logged in

fixed backend error message if invited user does not exist

8183e2d

eternalmaha merged commit 80752a4 into main Sep 1, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

RossaMania commented Aug 28, 2024 •

edited by eternalmaha

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

RossaMania commented Aug 28, 2024 •

edited

Loading

RossaMania commented Aug 28, 2024 •

edited

Loading

bbland1 Aug 29, 2024

RossaMania Aug 29, 2024

eternalmaha Aug 30, 2024

bbland1 Aug 29, 2024

tannaurus Aug 31, 2024

tannaurus Aug 31, 2024

bbland1 Aug 29, 2024 •

edited

Loading

RossaMania Aug 29, 2024

eternalmaha Aug 30, 2024

bbland1 Aug 29, 2024

RossaMania Aug 29, 2024

bbland1 commented Aug 29, 2024

eternalmaha commented Aug 29, 2024 •

edited

Loading

RossaMania commented Aug 29, 2024

bbland1 commented Aug 29, 2024

eternalmaha commented Aug 30, 2024

RossaMania commented Aug 30, 2024

bbland1 Aug 31, 2024

zahrafalak commented Aug 31, 2024

tannaurus Aug 31, 2024 •

edited

Loading

RossaMania Aug 31, 2024 •

edited

Loading

tannaurus Aug 31, 2024

RossaMania Sep 1, 2024

RossaMania commented Aug 31, 2024

tannaurus Aug 31, 2024

tannaurus Aug 31, 2024

RossaMania Aug 31, 2024

RossaMania Aug 31, 2024

eternalmaha Sep 1, 2024

eternalmaha commented Sep 1, 2024


		const trimmedEmailName = validateTrimmedString(emailName);

		if (!trimmedEmailName) {

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

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

Conversation

RossaMania commented Aug 28, 2024 • edited by eternalmaha Loading

Description

Related Issue

Acceptance Criteria

Type of Changes

Updates

Before

After

Testing Steps / QA Criteria

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

RossaMania commented Aug 28, 2024 • edited Loading

RossaMania commented Aug 28, 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

Choose a reason for hiding this comment

bbland1 Aug 29, 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

bbland1 commented Aug 29, 2024

eternalmaha commented Aug 29, 2024 • edited Loading

RossaMania commented Aug 29, 2024

bbland1 commented Aug 29, 2024

eternalmaha commented Aug 30, 2024

RossaMania commented Aug 30, 2024

Choose a reason for hiding this comment

zahrafalak commented Aug 31, 2024

tannaurus Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

RossaMania Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossaMania commented Aug 31, 2024

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

eternalmaha commented Sep 1, 2024

RossaMania commented Aug 28, 2024 •

edited by eternalmaha

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading

RossaMania commented Aug 28, 2024 •

edited

Loading

RossaMania commented Aug 28, 2024 •

edited

Loading

bbland1 Aug 29, 2024 •

edited

Loading

eternalmaha commented Aug 29, 2024 •

edited

Loading

tannaurus Aug 31, 2024 •

edited

Loading

RossaMania Aug 31, 2024 •

edited

Loading