Show AC unread indicator with counter and seen state color (#15304)

- Display Activity Center unread badge with the unread counter. - Use the new seen state stored in `status-go` to change the color of the notification. - Performance: split the `top-nav` component into left and right section components and render the unread indicator in a separate component to not trigger the re-render of the entire `top-nav` (as was before). Fixes https://github.com/status-im/status-mobile/issues/14851 Demo: https://user-images.githubusercontent.com/46027/224299978-770dd5f1-302b-4375-af2b-3cd181ffdc9d.webm Notes ===== - Fix/improve: `quo/counter` displayed `NaN` to the user if the input value was an empty string. - In Figma, there's a border around the unread indicator. I didn't implement this because the ideal solution IMO involves changing the `quo/counter` component a little bit because the width of the component varies according to the content displayed (1, 9, 99, 100, etc) and I wanted to the right thing in a separate PR. Design notes ============ There's an ongoing conversation with the Design team to decide what to do with the gray indicator on top of the bell icon, since there's little contrast when it's is in the `seen` state. Platforms ========= - Android - iOS Steps to test ============= - Open Status - Receive one or more notifications in the Home screen and check the unread indicator is blue and has a counter. - Open the AC and close it, notice the unread indicator is now in the `seen` state. You can close the app and re-open and the state is persisted. - Mark notifications as read/unread at will, check the unread counter is correct.
2025-02-23 22:08:46 +00:00 · 2023-03-15 12:41:34 -03:00 · 2023-03-15 12:41:34 -03:00 · e8556a9abf
commit e8556a9abf
parent f640eb8c8f
14 changed files with 279 additions and 139 deletions
--- a/src/quo2/components/counter/tests/counter_component_spec.cljs
+++ b/src/quo2/components/counter/tests/counter_component_spec.cljs
@ -1,34 +1,29 @@
 (ns quo2.components.counter.--tests--.counter-component-spec
-  (:require ["@testing-library/react-native" :as rtl]
-            [quo2.components.counter.counter :as counter]
-            [reagent.core :as reagent]))
+  (:require [quo2.components.counter.counter :as counter]
+            [test-helpers.component :as h]))

-(defn render-counter
-  ([]
-   (render-counter {} nil))
-  ([opts value]
-   (rtl/render (reagent/as-element [counter/counter opts value]))))
+(h/describe "counter component"
+  (h/test "default render of counter component"
+    (h/render [counter/counter {} nil])
+    (-> (h/expect (h/get-by-test-id :counter-component))
+        (h/is-truthy)))

-(js/global.test "default render of counter component"
-  (fn []
-    (render-counter)
-    (-> (js/expect (rtl/screen.getByTestId "counter-component"))
-        (.toBeTruthy))))
+  (h/test "renders counter with a string value"
+    (h/render [counter/counter {} "1"])
+    (-> (h/expect (h/get-by-text "1"))
+        (h/is-truthy)))

-(js/global.test "renders counter with a string value"
-  (fn []
-    (render-counter {} "1")
-    (-> (js/expect (rtl/screen.getByText "1"))
-        (.toBeTruthy))))
+  (h/test "renders counter with an integer value"
+    (h/render [counter/counter {} 1])
+    (-> (h/expect (h/get-by-text "1"))
+        (h/is-truthy)))

-(js/global.test "renders counter with an integer value"
-  (fn []
-    (render-counter {} 1)
-    (-> (js/expect (rtl/screen.getByText "1"))
-        (.toBeTruthy))))
+  (h/test "renders counter with max value 99+ by default"
+    (h/render [counter/counter {} 100])
+    (-> (h/expect (h/get-by-text "99+"))
+        (h/is-truthy)))

-(js/global.test "renders counter with value 99+ when the value is greater than 99"
-  (fn []
-    (render-counter {} "100")
-    (-> (js/expect (rtl/screen.getByText "99+"))
-        (.toBeTruthy))))
+  (h/test "renders counter with custom max value when set to 150"
+    (h/render [counter/counter {:max-value 150} 151])
+    (-> (h/expect (h/get-by-text "150+"))
+        (h/is-truthy))))
--- a/src/quo2/components/counter/counter.cljs
+++ b/src/quo2/components/counter/counter.cljs
@ -1,8 +1,10 @@
 (ns quo2.components.counter.counter
-  (:require [quo2.components.markdown.text :as text]
-            [quo2.foundations.colors :as colors]
-            [quo2.theme :as theme]
-            [react-native.core :as rn]))
+  (:require
+    [quo2.components.markdown.text :as text]
+    [quo2.foundations.colors :as colors]
+    [quo2.theme :as theme]
+    [react-native.core :as rn]
+    [utils.number :as utils-number]))

 (def themes
  {:light {:default   colors/primary-50
@ -19,9 +21,9 @@
  (get-in themes [(theme/get-theme) key]))

 (defn counter
-  "type:    default, secondary, grey, outline
-   value:   integer"
-  [{:keys [type override-text-color override-bg-color style accessibility-label]} value]
+  [{:keys [type override-text-color override-bg-color style accessibility-label max-value]
+    :or   {max-value 99}}
+   value]
  (let [type       (or type :default)
        text-color (or override-text-color
                       (if (or
@ -29,11 +31,9 @@
                            (= type :default))
                         colors/white
                         colors/neutral-100))
-        value      (if (integer? value)
-                     value
-                     (js/parseInt value))
-        label      (if (> value 99)
-                     "99+"
+        value      (utils-number/parse-int value)
+        label      (if (> value max-value)
+                     (str max-value "+")
                     (str value))
        width      (case (count label)
                     1 16
@ -59,7 +59,7 @@
                                    (or override-bg-color
                                        (get-color type)))

-                             (> value 99)
+                             (> value max-value)
                             (assoc :padding-left 0.5))}
     [text/text
      {:weight :medium
--- a/src/status_im/data_store/activities.cljs
+++ b/src/status_im/data_store/activities.cljs
@ -66,3 +66,7 @@
      (update :message #(when % (messages/<-rpc %)))
      (update :reply-message #(when % (messages/<-rpc %)))
      (dissoc :chatId)))
+
+(defn <-rpc-seen-state
+  [item]
+  (:hasSeen item))
--- a/src/status_im/multiaccounts/login/core.cljs
+++ b/src/status_im/multiaccounts/login/core.cljs
@ -414,6 +414,7 @@
              (communities/fetch)
              (logging/set-log-level (:log-level multiaccount))
              (activity-center/notifications-fetch-pending-contact-requests)
+              (activity-center/update-seen-state)
              (activity-center/notifications-fetch-unread-count))))

 (re-frame/reg-fx
--- a/src/status_im/transport/message/core.cljs
+++ b/src/status_im/transport/message/core.cljs
@ -43,6 +43,7 @@
        ^js invitations                (.-invitations response-js)
        ^js removed-chats              (.-removedChats response-js)
        ^js activity-notifications     (.-activityCenterNotifications response-js)
+        ^js activity-center-state      (.-activityCenterState response-js)
        ^js pin-messages               (.-pinMessages response-js)
        ^js removed-messages           (.-removedMessages response-js)
        ^js visibility-status-updates  (.-statusUpdates response-js)
@ -75,6 +76,15 @@
                  (activity-center/show-toasts notifications)
                  (process-next response-js sync-handler)))

+      (some? activity-center-state)
+      (let [seen? (-> activity-center-state
+                      types/js->clj
+                      data-store.activities/<-rpc-seen-state)]
+        (js-delete response-js "activityCenterState")
+        (rf/merge cofx
+                  (activity-center/reconcile-seen-state seen?)
+                  (process-next response-js sync-handler)))
+
      (seq installations)
      (let [installations-clj (types/js->clj installations)]
        (js-delete response-js "installations")
--- a/src/status_im2/common/home/style.cljs
+++ b/src/status_im2/common/home/style.cljs
@ -13,3 +13,34 @@
   :margin-right        6
   :weight              :semi-bold
   :size                :heading-1})
+
+(defn unread-indicator
+  [unread-count max-value]
+  (let [right-offset (cond
+                       (> unread-count max-value)
+                       -14
+
+                       ;; Greater than 9 means we'll need 2 digits to represent
+                       ;; the text.
+                       (> unread-count 9)
+                       -10
+
+                       :else -6)]
+    {:position :absolute
+     :top      -6
+     :right    right-offset
+     :z-index  4}))
+
+(def left-section
+  {:position :absolute
+   :left     20
+   :top      12})
+
+(def right-section
+  {:position       :absolute
+   :right          20
+   :top            12
+   :flex-direction :row})
+
+(def top-nav-container
+  {:height 56})
--- a/src/status_im2/common/home/view.cljs
+++ b/src/status_im2/common/home/view.cljs
@ -1,11 +1,12 @@
 (ns status-im2.common.home.view
-  (:require [quo2.core :as quo]
-            [quo2.foundations.colors :as colors]
-            [react-native.core :as rn]
-            [react-native.hole-view :as hole-view]
-            [status-im2.common.home.style :as style]
-            [status-im2.common.plus-button.view :as components.plus-button]
-            [utils.re-frame :as rf]))
+  (:require
+    [quo2.core :as quo]
+    [quo2.foundations.colors :as colors]
+    [react-native.core :as rn]
+    [status-im2.common.home.style :as style]
+    [status-im2.common.plus-button.view :as plus-button]
+    [status-im2.constants :as constants]
+    [utils.re-frame :as rf]))

 (defn title-column
  [{:keys [label handler accessibility-label]}]
@ -13,7 +14,7 @@
   [rn/view {:flex 1}
    [quo/text style/title-column-text
     label]]
-   [components.plus-button/plus-button
+   [plus-button/plus-button
    {:on-press            handler
     :accessibility-label accessibility-label}]])

@ -30,84 +31,65 @@
     :override-background-color (when (and dark? default?)
                                  colors/neutral-90)}))

-(defn- base-button
-  [icon on-press accessibility-label button-common-props]
-  [quo/button
-   (merge
-    {:on-press            on-press
-     :accessibility-label accessibility-label}
-    button-common-props)
-   icon])
+(defn- unread-indicator
+  []
+  (let [unread-count (rf/sub [:activity-center/unread-count])
+        indicator    (rf/sub [:activity-center/unread-indicator])
+        unread-type  (case indicator
+                       :unread-indicator/seen :grey
+                       :unread-indicator/new  :default
+                       nil)]
+    (when (pos? unread-count)
+      [quo/counter
+       {:accessibility-label :activity-center-unread-count
+        :type                unread-type
+        :style               (style/unread-indicator unread-count
+                                                     constants/activity-center-max-unread-count)}
+       unread-count])))
+
+(defn- left-section
+  [{:keys [avatar]}]
+  [rn/touchable-without-feedback {:on-press #(rf/dispatch [:navigate-to :my-profile])}
+   [rn/view
+    {:accessibility-label :open-profile
+     :style               style/left-section}
+    [quo/user-avatar
+     (merge {:status-indicator? true
+             :size              :small}
+            avatar)]]])
+
+(defn- right-section
+  [{:keys [button-type search?]}]
+  (let [button-common-props (get-button-common-props button-type)]
+    [rn/view {:style style/right-section}
+     (when search?
+       [quo/button
+        (assoc button-common-props :accessibility-label :open-search-button)
+        :i/search])
+     [quo/button
+      (assoc button-common-props :accessibility-label :open-scanner-button)
+      :i/scan]
+     [quo/button
+      (assoc button-common-props :accessibility-label :show-qr-button)
+      :i/qr-code]
+     [rn/view
+      [unread-indicator]
+      [quo/button
+       (merge button-common-props
+              {:accessibility-label :open-activity-center-button
+               :on-press            #(rf/dispatch [:activity-center/open])})
+       :i/activity-center]]]))

 (defn top-nav
-  "[top-nav opts]
-  opts
-  {:type                   :default/:blurred/:shell
-   :style                  override-style
-   :avatar                 user-avatar}
+  "[top-nav props]
+  props
+  {:type    quo/button types
+   :style   override-style
+   :avatar  user-avatar
+   :search? When non-nil, show search button}
  "
-  [{:keys [type style avatar search?] :or {type :default}}]
-  (let [button-common-props    (get-button-common-props type)
-        notif-count            (rf/sub [:activity-center/unread-count])
-        new-notifications?     (pos? notif-count)
-        notification-indicator :unread-dot
-        counter-label          "0"]
-    [rn/view {:style (assoc style :height 56)}
-     ;; Left Section
-     [rn/touchable-without-feedback {:on-press #(rf/dispatch [:navigate-to :my-profile])}
-      [rn/view
-       {:accessibility-label :open-profile
-        :style               {:position :absolute
-                              :left     20
-                              :top      12}}
-       [quo/user-avatar
-        (merge
-         {:status-indicator? true
-          :size              :small}
-         avatar)]]]
-     ;; Right Section
-     [rn/view
-      {:style {:position       :absolute
-               :right          20
-               :top            12
-               :flex-direction :row}}
-      (when search?
-        [base-button :i/search #() :open-search-button button-common-props])
-      [base-button :i/scan #() :open-scanner-button button-common-props]
-      [base-button :i/qr-code #() :show-qr-button button-common-props]
-      [rn/view                     ;; Keep view instead of "[:<>" to make sure relative
-       ;; position is calculated from this view instead of its parent
-       [hole-view/hole-view
-        {:key   new-notifications? ;; Key is required to force removal of holes
-         :holes (cond
-                  (not new-notifications?) ;; No new notifications, remove holes
-                  []
-
-                  (= notification-indicator :unread-dot)
-                  [{:x 37 :y -3 :width 10 :height 10 :borderRadius 5}]
-
-                  :else
-                  [{:x 33 :y -7 :width 18 :height 18 :borderRadius 7}])}
-        [base-button :i/activity-center #(rf/dispatch [:activity-center/open])
-         :open-activity-center-button button-common-props]]
-       (when new-notifications?
-         (if (= notification-indicator :counter)
-           [quo/counter
-            {:accessibility-label :notifications-unread-badge
-             :outline             false
-             :override-text-color colors/white
-             :override-bg-color   colors/primary-50
-             :style               {:position :absolute
-                                   :left     34
-                                   :top      -6}}
-            counter-label]
-           [rn/view
-            {:accessible          true
-             :accessibility-label :notifications-unread-badge
-             :style               {:width            8
-                                   :height           8
-                                   :border-radius    4
-                                   :top              -2
-                                   :left             38
-                                   :position         :absolute
-                                   :background-color colors/primary-50}}]))]]]))
+  [{:keys [type style avatar search?]
+    :or   {type :default}}]
+  [rn/view {:style (merge style/top-nav-container style)}
+   [left-section {:avatar avatar}]
+   [right-section {:button-type type :search? search?}]])
--- a/src/status_im2/constants.cljs
+++ b/src/status_im2/constants.cljs
@ -37,6 +37,7 @@
 (def ^:const activity-center-membership-status-declined 3)

 (def ^:const activity-center-mark-all-as-read-undo-time-limit-ms 4000)
+(def ^:const activity-center-max-unread-count 99)

 (def ^:const emoji-reaction-love 1)
 (def ^:const emoji-reaction-thumbs-up 2)
--- a/src/status_im2/contexts/activity_center/events.cljs
+++ b/src/status_im2/contexts/activity_center/events.cljs
@ -9,8 +9,7 @@
            [taoensso.timbre :as log]
            [utils.collection :as collection]
            [utils.i18n :as i18n]
-            [utils.re-frame :as rf]
-            [status-im2.navigation.events :as navigation]))
+            [utils.re-frame :as rf]))

 (def defaults
  {:filter-status          :unread
@ -24,15 +23,18 @@

 (rf/defn open-activity-center
  {:events [:activity-center/open]}
-  [{:keys [db] :as cofx} {:keys [filter-type filter-status]}]
-  (rf/merge cofx
-            {:db (cond-> db
-                   filter-status
-                   (assoc-in [:activity-center :filter :status] filter-status)
+  [{:keys [db]} {:keys [filter-type filter-status]}]
+  {:db             (cond-> db
+                     filter-status
+                     (assoc-in [:activity-center :filter :status] filter-status)

-                   filter-type
-                   (assoc-in [:activity-center :filter :type] filter-type))}
-            (navigation/open-modal :activity-center {})))
+                     filter-type
+                     (assoc-in [:activity-center :filter :type] filter-type))
+   :dispatch       [:open-modal :activity-center {}]
+   ;; We delay marking as seen so that the user doesn't see the unread bell icon
+   ;; change while the Activity Center modal is opening.
+   :dispatch-later [{:ms       1000
+                     :dispatch [:activity-center/mark-as-seen]}]})

 ;;;; Misc

@ -48,7 +50,7 @@
       (filter #(= notification-id (:id %)))
       first))

-;;;; Notification reconciliation
+;;;; Reconciliation

 (defn- update-notifications
  [db-notifications new-notifications {filter-type :type filter-status :status}]
@ -97,6 +99,11 @@
                    (remove #(activities/pending-contact-request? contact-id %)
                            notifications)))})

+(rf/defn reconcile-seen-state
+  {:events [:activity-center/reconcile-seen-state]}
+  [{:keys [db]} seen?]
+  {:db (assoc-in db [:activity-center :seen?] seen?)})
+
 ;;;; Status changes (read/dismissed/deleted)

 (rf/defn mark-as-read
@ -451,6 +458,50 @@

 ;;;; Unread counters

+(rf/defn update-seen-state
+  {:events [:activity-center/update-seen-state]}
+  [_]
+  {:json-rpc/call
+   [{:method     "wakuext_hasUnseenActivityCenterNotifications"
+     :params     []
+     :on-success #(rf/dispatch [:activity-center/update-seen-state-success %])
+     :on-error   #(rf/dispatch [:activity-center/update-seen-state-error %])}]})
+
+(rf/defn update-seen-state-success
+  {:events [:activity-center/update-seen-state-success]}
+  [{:keys [db]} unseen?]
+  {:db (assoc-in db [:activity-center :seen?] (not unseen?))})
+
+(rf/defn update-seen-state-error
+  {:events [:activity-center/update-seen-state-error]}
+  [_ error]
+  (log/error "Failed to update Activity Center seen state"
+             {:error error
+              :event :activity-center/update-seen-state}))
+
+(rf/defn mark-as-seen
+  {:events [:activity-center/mark-as-seen]}
+  [_]
+  {:json-rpc/call
+   [{:method     "wakuext_markAsSeenActivityCenterNotifications"
+     :params     []
+     :on-success #(rf/dispatch [:activity-center/mark-as-seen-success %])
+     :on-error   #(rf/dispatch [:activity-center/mark-as-seen-error %])}]})
+
+(rf/defn mark-as-seen-success
+  {:events [:activity-center/mark-as-seen-success]}
+  [{:keys [db]} response]
+  {:db (assoc-in db
+        [:activity-center :seen?]
+        (get-in response [:activityCenterState :hasSeen]))})
+
+(rf/defn mark-as-seen-error
+  {:events [:activity-center/mark-as-seen-error]}
+  [_ error]
+  (log/error "Failed to mark Activity Center as seen"
+             {:error error
+              :event :activity-center/mark-as-seen}))
+
 (rf/defn notifications-fetch-unread-count
  {:events [:activity-center.notifications/fetch-unread-count]}
  [_]
--- a/src/status_im2/subs/activity_center.cljs
+++ b/src/status_im2/subs/activity_center.cljs
@ -34,6 +34,22 @@
        vals
        (reduce + 0))))

+(re-frame/reg-sub
+ :activity-center/seen?
+ :<- [:activity-center]
+ (fn [activity-center]
+   (:seen? activity-center)))
+
+(re-frame/reg-sub
+ :activity-center/unread-indicator
+ :<- [:activity-center/seen?]
+ :<- [:activity-center/unread-count]
+ (fn [[seen? unread-count]]
+   (cond
+     (zero? unread-count) :unread-indicator/none
+     seen?                :unread-indicator/seen
+     :else                :unread-indicator/new)))
+
 (re-frame/reg-sub
 :activity-center/mark-all-as-read-undoable-till
 :<- [:activity-center]
--- a/src/status_im2/subs/activity_center_test.cljs
+++ b/src/status_im2/subs/activity_center_test.cljs
@ -63,3 +63,25 @@
     types/admin                7})

  (is (= 28 (rf/sub [sub-name]))))
+
+(h/deftest-sub :activity-center/unread-indicator
+  [sub-name]
+  (testing "not seen and no unread notifications"
+    (swap! rf-db/app-db assoc-in [:activity-center :unread-counts-by-type] {types/one-to-one-chat 0})
+    (swap! rf-db/app-db assoc-in [:activity-center :seen?] false)
+    (is (= :unread-indicator/none (rf/sub [sub-name]))))
+
+  (testing "not seen and one or more unread notifications"
+    (swap! rf-db/app-db assoc-in [:activity-center :unread-counts-by-type] {types/one-to-one-chat 1})
+    (swap! rf-db/app-db assoc-in [:activity-center :seen?] false)
+    (is (= :unread-indicator/new (rf/sub [sub-name]))))
+
+  (testing "seen and no unread notifications"
+    (swap! rf-db/app-db assoc-in [:activity-center :unread-counts-by-type] {types/one-to-one-chat 0})
+    (swap! rf-db/app-db assoc-in [:activity-center :seen?] true)
+    (is (= :unread-indicator/none (rf/sub [sub-name]))))
+
+  (testing "seen and one or more unread notifications"
+    (swap! rf-db/app-db assoc-in [:activity-center :unread-counts-by-type] {types/one-to-one-chat 1})
+    (swap! rf-db/app-db assoc-in [:activity-center :seen?] true)
+    (is (= :unread-indicator/seen (rf/sub [sub-name])))))
--- a/src/utils/number.cljs
+++ b/src/utils/number.cljs
@ -13,3 +13,13 @@
  (let [scale (Math/pow 10 decimal-places)]
    (/ (Math/round (* n scale))
       scale)))
+
+(defn parse-int
+  "Parses `n` as an integer. Defaults to zero or `default` instead of NaN."
+  ([n]
+   (parse-int n 0))
+  ([n default]
+   (let [maybe-int (js/parseInt n 10)]
+     (if (integer? maybe-int)
+       maybe-int
+       default))))
--- a/src/utils/number_test.cljs
+++ b/src/utils/number_test.cljs
@ -0,0 +1,17 @@
+(ns utils.number-test
+  (:require [cljs.test :refer [deftest is testing]]
+            [utils.number :as utils-number]))
+
+(deftest parse-int
+  (testing "defaults to zero"
+    (is (= 0 (utils-number/parse-int nil))))
+
+  (testing "accepts any other default value"
+    (is (= 3 (utils-number/parse-int "" 3)))
+    (is (= :invalid-int (utils-number/parse-int "" :invalid-int))))
+
+  (testing "valid numbers"
+    (is (= -6 (utils-number/parse-int "-6a" 0)))
+    (is (= 6 (utils-number/parse-int "6" 0)))
+    (is (= 6 (utils-number/parse-int "6.99" 0)))
+    (is (= -6 (utils-number/parse-int "-6" 0)))))
--- a/test/appium/views/home_view.py
+++ b/test/appium/views/home_view.py
@ -197,7 +197,7 @@ class HomeView(BaseView):

        # Notification centre
        self.notifications_button = Button(self.driver, accessibility_id="notifications-button")
-        self.notifications_unread_badge = BaseElement(self.driver, accessibility_id="notifications-unread-badge") 
+        self.notifications_unread_badge = BaseElement(self.driver, accessibility_id="activity-center-unread-count")
        self.open_activity_center_button = Button(self.driver, accessibility_id="open-activity-center-button")
        self.close_activity_centre = Button(self.driver, accessibility_id="close-activity-center")