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

scp-2091: Marlowe Dashboard Client: Implement toast notification #3015

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

hrajchert
Copy link
Contributor

@hrajchert hrajchert commented Apr 14, 2021

This PR implements toast notification for success and error messages.

Successful mobile

Successful desktop

toast_succesfull_desktop

Error mobile collapsed

Error mobile expanded

Error desktop collapsed

errortoast_desktop_collapsed

Error desktop expanded

error_toast_desktop_expanded

Pre-submit checklist:

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Pre-merge checklist:

  • Someone approved it
  • Commits have useful messages
  • Review clarifications made it into the code
  • History is moderately tidy; or going to squash-merge

@hrajchert hrajchert requested a review from merivale April 14, 2021 21:42
Copy link
Contributor

@merivale merivale left a comment

Choose a reason for hiding this comment

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

The toast looks really cool. 😎 I'm curious to try out the MainFrameLoop idea on other things (notably cards). This might be quite a nice approach.

Comment on lines +268 to +269
let
networkErrorToast = \_ -> addToast $ connectivityErrorToast "Couldn't generate wallet"
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this (and line 234 above) should be factored out somehow, but I'm not immediately sure how. Just a note for the future...

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.

None yet

2 participants