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

Implement Toast Notification #69

Merged
merged 10 commits into from
Oct 15, 2024
Merged

Implement Toast Notification #69

merged 10 commits into from
Oct 15, 2024

Conversation

warrenchan13
Copy link
Collaborator

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

Description

This code provides alerts for important actions and notifications in a non-intrusive manner using toastify.

Related Issue

Closes #55

Acceptance Criteria

  • Notification Types: The application should support different types of notifications (e.g., success, error, info, warning).
  • Positioning: Notifications should appear at the top-right corner of the screen (or as specified) to ensure they are easily noticeable without obstructing content.
  • Duration: Notifications should automatically disappear after a set duration (e.g., 3-5 seconds) unless the user dismisses them.
  • Custom Messages: The user should be able to see customizable messages in the toast notifications (e.g., "Item added successfully!", "Error: Unable to delete the item.").
  • Accessibility: The toast notifications should be accessible, allowing screen readers to read the messages.
  • Dismiss Action: Users should have the option to manually dismiss notifications by clicking an “X” button on the toast.
  • Animation: Notifications should fade in and out smoothly to enhance user experience.

Type of Changes

enhancement

Updates

Before

Screenshot 2024-10-01 at 1 49 01 PM
Screenshot 2024-10-01 at 1 49 20 PM

After

Screenshot 2024-10-01 at 1 49 51 PM
Screenshot 2024-10-01 at 1 50 19 PM
Screenshot 2024-10-01 at 1 50 43 PM

Testing Steps / QA Criteria

  1. Ran NPM and signed in
  2. Make sure Toastify works on key actions

Copy link

github-actions bot commented Oct 1, 2024

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

https://tcl-75-smart-shopping-list--pr69-wc-toastify-notifica-urvhwya5.web.app

(expires Sun, 20 Oct 2024 21:38:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 1f1fd53c369e1fa31e15c310aa075b4e8f4f8dde

@warrenchan13
Copy link
Collaborator Author

Since we replaced the "alerts" with Toastify, is there a way to set up tests for Toastify?

@kweeuhree
Copy link
Collaborator

kweeuhree commented Oct 1, 2024

Looks really cool, I love this change! Makes such a big difference for the app!

I noticed that a confirm window is removed from handleDeleteItem inside ListItem component, so we should add that back

kweeuhree
kweeuhree previously approved these changes Oct 1, 2024
@kweeuhree kweeuhree dismissed their stale review October 1, 2024 21:29

noticed absence of a confirm window

@kweeuhree
Copy link
Collaborator

I noticed that you added back the if block and voted for the custom dialog! Do you think you'd want to create that dialog? Or we can team up, let me know!
Also, it looks like tests do not pass. Have you tried adding that 'spy' that the error message is talking about? I will take a look at updating the tests in a bit

src/App.jsx Outdated Show resolved Hide resolved
src/views/Home.jsx Outdated Show resolved Hide resolved
Copy link
Member

@deeheber deeheber left a comment

Choose a reason for hiding this comment

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

We discussed in our sync that it's ok for @warrenchan13 to remove the test portion of this to move forward.

The other option is to add mocks for this library for tests to pass.

Copy link
Collaborator

@jendevelops jendevelops left a comment

Choose a reason for hiding this comment

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

As @deeheber said, just need to update the tests to get this baby merged in!

tests/List.test.jsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@jendevelops jendevelops left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the tests!

@warrenchan13 warrenchan13 merged commit 2a8dc17 into main Oct 15, 2024
6 checks passed
@warrenchan13 warrenchan13 deleted the wc-toastify-notification branch October 15, 2024 06:40
@kweeuhree kweeuhree restored the wc-toastify-notification branch October 18, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

15. As a user, I want to receive alerts for important actions and notifications in a non-intrusive manner
5 participants