Add persistent notification with dismiss and action button #611
Reference in New Issue
Block a user
Delete Branch "feature/persistent-notification"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
closes https://github.com/maybe-finance/maybe/issues/597
Maybe notification partial is becoming a little complex ... Do you want me to rework code ? create partials , helpers ?
Looking good, thanks for tackling this one!
Just a few things that I think we need to address before merging:
Styling
There are a few inconsistencies between this and the design file (also check spelling):
vs.
Here's the official spec for this one: https://www.figma.com/file/lonJmVk3HYkwZoIO7xYP2w/Maybe-App-(Community)?node-id=2568%3A2765&mode=dev
I18n
For default messages like "Dismiss" in the
_notificationpartial, I think we should add to a localization file.@@ -1,23 +1,42 @@<%# locals: (type: "success", content:) -%><%# locals: (type: "success", content: { title: '', body: ''}, action: { label:'' , url:'' }, options: { auto_dismiss: true }) -%>I think we will generally want to avoid dynamic Tailwind class strings like this so the JIT compiler can recognize the classes that need to be included:
https://tailwindcss.com/docs/content-configuration#dynamic-class-names
Thanks for the review. I've tried to reproduce the spec, I hope it will be better.
Nice, looks good now!