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

Installed MUI and display popup modal when shopping list is empty #25

Merged
merged 6 commits into from
Sep 10, 2024

Conversation

sar-mko
Copy link
Collaborator

@sar-mko sar-mko commented Sep 3, 2024

Description

This added an alert to the List page, for users without any items. The popup message alerts them to add items to the Manage List page, and includes a button to navigate them there. We installed the Material UI dependency and used their popup model, and we leveraged conditional rendering to determine when the popup model shows up.

Related Issue

Closes #8

Acceptance Criteria

  • The list view, when there are no items to display, should show a prompt (e.g., a button) for the user to add their first item

Type of Changes

feature

Updates

After

Screenshot 2024-09-03 at 10 47 55 AM

Testing Steps / QA Criteria

  • use npm i to install MUI
  • Create a new shopping list, that should be empty
  • When redirected to the list page, a popup window should open
  • Button on pop up window sends to you manage list
  • If list is still empty, pop up window should still display until items are added
  • Should be able to click out of pop up window

Copy link

github-actions bot commented Sep 3, 2024

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

https://tcl-76-smart-shopping-list--pr25-sm-eb-create-welcome-sg37cw5r.web.app

(expires Sun, 15 Sep 2024 05:52:49 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 512b1a88be8ae05fd3e727b99332819df760271d

Copy link
Collaborator

@MarcosPerez16 MarcosPerez16 left a comment

Choose a reason for hiding this comment

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

After reviewing and testing the PR, I found that I was not seeing the Tip information when im redicted to the List view. I created a list, it then redirected me, but only shows "Go To Manage List" on my screen. I will provide a snippet of what I am seeing.
NotSeeingTip

@MarcosPerez16
Copy link
Collaborator

I made sure MUI was installed, I thought that was the issue, but let me know if anyone else comes across this issue.

@EmmaBin
Copy link
Collaborator

EmmaBin commented Sep 5, 2024

I made sure MUI was installed, I thought that was the issue, but let me know if anyone else comes across this issue.

Hi Marcos, the reason for only seeing the btn text, not the Tip information might be due to the dark mode you are using, let me see how to fix this.

I made sure MUI was installed, I thought that was the issue, but let me know if anyone else comes across this issue.

Copy link
Collaborator

@arandel1 arandel1 left a comment

Choose a reason for hiding this comment

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

Looks great! I was initially having trouble because I needed to npm i MUI. Everything works great!

@MarcosPerez16
Copy link
Collaborator

@sar-mko @EmmaBin I did a re-test and everything seems good on my end now. Nice job!

Copy link
Collaborator

@MarcosPerez16 MarcosPerez16 left a comment

Choose a reason for hiding this comment

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

Everything looks good after re-testing the code. Good work!

src/views/List.jsx Outdated Show resolved Hide resolved
fontSize: 20,
});

export default function BasicModal({ dataEmpty }) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion ( non-blocking ): Adding this modal in is very nice! Something to consider for the future is to make it more extensible. For example, you could pass in props like header, body, and buttonText and then use this throughout your codebase.

src/views/Modal.jsx Outdated Show resolved Hide resolved
@mindyzwan
Copy link
Member

It looks beautiful! Well done 👏

One item I noticed is that the pop-up from a new list being created hangs out in front of this modal and it does look a little odd to have a modal on top of a modal. This implementation functionally works and meets the acceptance criteria so I'm approving. We just may want to consider rethinking how that initial "your list has been created!" pop-up works at some point in the future. We have a couple weeks set aside to put finishing touches/styling on the app so we can always put it off a bit.

Screenshot 2024-09-06 at 9 09 34 AM

@sar-mko sar-mko requested a review from mindyzwan September 8, 2024 16:01
@mindyzwan
Copy link
Member

The changes look great! Well done! 👏

@sar-mko sar-mko merged commit 1b31ba0 into main Sep 10, 2024
2 checks passed
@sar-mko sar-mko deleted the sm-eb-create-welcome-prompt-issue-8 branch September 10, 2024 17:13
@mindyzwan mindyzwan restored the sm-eb-create-welcome-prompt-issue-8 branch October 19, 2024 06:59
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