[#14245] Fix read/unread notifications filter #14333
Conversation
rasom
requested review from
flexsurfer,
ilmotta and
smohamedjavid
and removed request for
flexsurfer
Jenkins BuildsClick to see older builds (12)
|
| [rn/view {:style {:width 300, :height 100}} | ||
| [rn/text | ||
| (gstring/format | ||
| "I exist just to avoid crashing for no reason. I'm sorry. Type %d" type)]]) |
There was a problem hiding this comment.
I think we don't need to tell Clojure devs that defmulti requires a default case, just like case, otherwise an exception is thrown if no match is found.
There was a problem hiding this comment.
It's just to emphasise without checking the code that this should be removed asap.
There was a problem hiding this comment.
It was funny to read it 😄 I just try to avoid comments explaining how something in the language works, so that's why I bothered you with this.
| @@ -25,6 +26,14 @@ | |||
|
|
|||
| (defmulti notification-component :type) | |||
|
|
|||
| ;; TODO(rasom): should be removed as soon as all notifications types covered | |||
| (defmethod notification-component :default | |||
There was a problem hiding this comment.
FYI: Although I wrote the solution with a multimethod, it'll die soon. In the branch I'm working on right now I had to split different notifications into different files for two reasons: code conflicts because we're touching the same file... and second and most important, because the identity verification notification has to be rendered also in a bottom sheet.
| (if (= filter-status :read) | ||
| "wakuext_readActivityCenterNotifications" | ||
| "wakuext_unreadActivityCenterNotifications")) | ||
| (def ^:const status-read 1) |
There was a problem hiding this comment.
Do you have a doc reference as to why we should tell the compiler these things are constants? Does it bring any meaningful performance improvement?
There was a problem hiding this comment.
Does it bring any meaningful performance improvement?
it doesn't
rasom
force-pushed
the
fix/14245-fix-read-unread-notifications-filter
branch
2 times, most recently
from
532aa0a
to
a7fe653
Compare
rasom
force-pushed
the
fix/14245-fix-read-unread-notifications-filter
branch
from
a7fe653
to
4353c4a
Compare
fix #14245
status: ready