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

Add notifications; render as pop-ups ("toasts") #226

Merged
merged 15 commits into from
Jul 31, 2023

Conversation

flybot-nam-nguyenhoai
Copy link
Contributor

@flybot-nam-nguyenhoai flybot-nam-nguyenhoai commented Jul 28, 2023

Closes #207

This pull request introduces pop-up ("toast") notifications and implements them for website errors (validation and HTTP).

  • Add generic notifications (as simple data maps).
    These notify users when their actions succeed or fail, and can be delivered in different ways (e.g., as pop-ups or printed to console output).

  • Notify when encountering errors.
    Only validation errors and HTTP failures are notified to users.

  • Render notifications as pop-up banners ("toasts").

Suggestions for future development

  • Notify on successful actions.

  • Make notification content user friendly.
    For example, error notifications in this PR contain plain Clojure maps of error details—they should instead contain helpful messages in natural language.

This commit introduces a new field, `:app/notification`, for generic
notifications that can be rendered in different ways, e.g., as pop-ups
or console print output.

Notifications are now sent for HTTP failures and validation errors.
This commit attaches a UUID to each app notification, so that
subscriptions to `:app/notification` can process successive identical
notifications (e.g., repeated HTTP failures) correctly.
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as draft July 28, 2023 09:10
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as ready for review July 28, 2023 09:11
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as draft July 28, 2023 09:21
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as ready for review July 28, 2023 09:28
Copy link
Owner

@skydread1 skydread1 left a comment

Choose a reason for hiding this comment

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

Good.

We need to add a cljs test for the notification.

client/common/src/flybot/client/common/db/event.cljs Outdated Show resolved Hide resolved
resources/public/index.html Show resolved Hide resolved
client/web/src/flybot/client/web/core/dom/page.cljs Outdated Show resolved Hide resolved
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as draft July 31, 2023 01:01
This commit makes the React components for pop-up notifications
common across all pages, similar to the header and footer components.
@flybot-nam-nguyenhoai
Copy link
Contributor Author

Tests have been added for generic notifications (i.e., notifications that are in plain Clojure map form and stored in :app/notification).

@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as ready for review July 31, 2023 03:38
@skydread1 skydread1 self-requested a review July 31, 2023 04:57
@skydread1 skydread1 self-assigned this Jul 31, 2023
@skydread1 skydread1 self-requested a review July 31, 2023 04:59
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as draft July 31, 2023 05:09
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai marked this pull request as ready for review July 31, 2023 05:36
@skydread1 skydread1 self-requested a review July 31, 2023 05:38
@skydread1 skydread1 merged commit 7330a6c into skydread1:master Jul 31, 2023
2 checks passed
@flybot-nam-nguyenhoai flybot-nam-nguyenhoai deleted the 207-ui-notifications branch July 31, 2023 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error messages are not user friendly
2 participants