Skip to content

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?

@kolesnikova-dev
Copy link
Collaborator

kolesnikova-dev 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

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

noticed absence of a confirm window

@kolesnikova-dev
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

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!

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
@kolesnikova-dev kolesnikova-dev 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