-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: Update the enable-notifications flows and notification settings UI #22524
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
Changes from all commits
779355a
b8d12b9
8f6a803
0273a1f
4c0c9af
18bc5da
205d3a4
cd4a61a
ef2d132
f254fa0
eea665b
b1d2727
5035f1a
e585136
8ba6578
fc6326e
5ae114b
8a53015
a7cbcfa
51a73b0
9cb5928
64adafb
2c477ad
a32f148
44a9533
a2735f2
4ea25cf
00a3218
2df1099
a66ca16
a5bdf6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,6 +107,7 @@ | |
| (let [new-account? (get db :onboarding/new-account?) | ||
| app-in-background-since (get db :app-in-background-since) | ||
| signed-up? (get-in db [:profile/profile :signed-up?]) | ||
| notifications-settings? (= (:view-id db) :screen/settings.notifications) | ||
| requires-bio-auth (and | ||
| signed-up? | ||
| (= (:auth-method db) "biometric") | ||
|
|
@@ -121,7 +122,9 @@ | |
| #(when-let [chat-id (:current-chat-id db)] | ||
| {:dispatch [:chat/mark-all-as-read chat-id]}) | ||
| #(when requires-bio-auth | ||
| {:dispatch [:biometric/authenticate {:on-fail on-biometric-auth-fail}]})))) | ||
| {:dispatch [:biometric/authenticate {:on-fail on-biometric-auth-fail}]}) | ||
| #(when notifications-settings? | ||
| {:dispatch [:notifications/check-notifications-blocked]})))) | ||
|
Comment on lines
+125
to
+127
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we're conditionally dispatching the |
||
|
|
||
| (rf/defn on-going-in-background | ||
| [{:keys [db now]}] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| (ns react-native.permissions | ||
| (:require | ||
| ["react-native-permissions" :refer | ||
| [check checkNotifications PERMISSIONS requestMultiple | ||
| [check checkNotifications openSettings PERMISSIONS requestMultiple | ||
| requestNotifications RESULTS]] | ||
| [clojure.string :as string] | ||
| [promesa.core :as promesa] | ||
|
|
@@ -95,3 +95,7 @@ | |
| [] | ||
| (-> (checkNotifications) | ||
| (promesa/then notification-permissions->notification-permission-statuses))) | ||
|
|
||
| (defn open-notification-settings | ||
| [] | ||
| (openSettings "notifications")) | ||
|
Comment on lines
+98
to
+101
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we're creating a function that should open the OS system settings screen for the app, and if possible it will display the notification related settings for the app (from the OS System pov, not inside Status app). |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| (ns status-im.common.kv-storage.effects | ||
| (:require | ||
| [react-native.mmkv :as mmkv] | ||
| [utils.re-frame :as rf])) | ||
|
|
||
| (rf/reg-fx :effects.kv/set-object | ||
| (fn [{:keys [id value]}] | ||
| (mmkv/set-object id value))) | ||
|
|
||
| (rf/reg-fx :effects.kv/merge-object | ||
| (fn [{key-id :key | ||
| :keys [value]}] | ||
| (let [kv-object (mmkv/get-object key-id {})] | ||
| (mmkv/set-object key-id | ||
| (merge kv-object value))))) | ||
|
|
||
| (rf/reg-fx :effects.kv/delete-key | ||
| (fn [key-id] | ||
| (mmkv/delete-key key-id))) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,25 +22,28 @@ | |
| [insets] | ||
| (let [supported-biometric-type (rf/sub [:biometrics/supported-type]) | ||
| bio-type-label (biometric/get-label-by-type supported-biometric-type) | ||
| profile-color (or (:color (rf/sub [:onboarding/profile])) | ||
| onboarding-profile (rf/sub [:onboarding/profile]) | ||
| profile-color (or (:color onboarding-profile) | ||
| (rf/sub [:profile/customization-color])) | ||
| syncing? (= (rf/sub [:view-id]) :screen/onboarding.syncing-biometric) | ||
| syncing? (:syncing? onboarding-profile) | ||
| biometric-type (rf/sub [:biometrics/supported-type])] | ||
| [rn/view {:style (style/buttons insets)} | ||
| [quo/button | ||
| {:size 40 | ||
| :accessibility-label :enable-biometrics-button | ||
| :icon-left (biometric/get-icon-by-type biometric-type) | ||
| :customization-color profile-color | ||
| :on-press #(rf/dispatch [:onboarding/enable-biometrics])} | ||
| :on-press #(rf/dispatch [:onboarding/biometrics-setup-start | ||
| {:enable-biometrics? true | ||
| :syncing? syncing?}])} | ||
|
Comment on lines
+36
to
+38
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we're using a different pattern for dispatching events from the onboarding screens. Instead of dispatching different events from the screen, we attempt to dispatch the same event with different parameters. This pattern is an attempt to simplify how we manage the onboarding screens, for example, if we choose to expand the onboarding steps, it would be ideal that we can refactor mostly the event layer instead of needing to refactor both the event layer and view layer. |
||
| (i18n/label :t/biometric-enable-button {:bio-type-label bio-type-label})] | ||
| [quo/button | ||
| {:accessibility-label :maybe-later-button | ||
| :background :blur | ||
| :type :grey | ||
| :on-press #(rf/dispatch (if syncing? | ||
| [:onboarding/finish-onboarding false] | ||
| [:onboarding/create-account-and-login])) | ||
| :on-press #(rf/dispatch [:onboarding/biometrics-setup-start | ||
| {:enable-biometrics? false | ||
| :syncing? syncing?}]) | ||
| :container-style {:margin-top 12}} | ||
| (i18n/label :t/maybe-later)]])) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,14 @@ | ||
| (ns status-im.contexts.onboarding.enable-notifications.view | ||
| (:require | ||
| [quo.context] | ||
| [quo.core :as quo] | ||
| [react-native.core :as rn] | ||
| [react-native.platform :as platform] | ||
| [react-native.safe-area :as safe-area] | ||
| [status-im.common.resources :as resources] | ||
| [status-im.contexts.onboarding.common.background.view :as background] | ||
| [status-im.contexts.onboarding.enable-notifications.style :as style] | ||
| [status-im.feature-flags :as ff] | ||
| [utils.i18n :as i18n] | ||
| [utils.re-frame :as rf])) | ||
|
|
||
|
|
@@ -17,51 +21,111 @@ | |
| :description (i18n/label :t/enable-notifications-sub-title) | ||
| :description-accessibility-label :notifications-sub-title}]) | ||
|
|
||
| (defn enable-notification-buttons | ||
| [{:keys [insets]}] | ||
| (let [profile-color (rf/sub [:onboarding/customization-color]) | ||
| ask-permission (fn [] | ||
| (rf/dispatch | ||
| [:request-notifications | ||
| {:on-allowed (fn [] | ||
| (js/setTimeout | ||
| #(rf/dispatch [:onboarding/finish-onboarding true]) | ||
| 300)) | ||
| :on-denied (fn [] | ||
| (js/setTimeout | ||
| #(rf/dispatch [:onboarding/finish-onboarding false]) | ||
| 300))}])) | ||
| skip-permission #(rf/dispatch [:onboarding/finish-onboarding false])] | ||
| [rn/view {:style (style/buttons insets)} | ||
| [quo/button | ||
| {:on-press ask-permission | ||
| :type :primary | ||
| :icon-left :i/notifications | ||
| :accessibility-label :enable-notifications-button | ||
| :customization-color profile-color} | ||
| (i18n/label :t/intro-wizard-title6)] | ||
| [quo/button | ||
| {:on-press skip-permission | ||
| :accessibility-label :enable-notifications-later-button | ||
| :type :grey | ||
| :background :blur | ||
| :container-style {:margin-top 12}} | ||
| (i18n/label :t/maybe-later)]])) | ||
| (defn on-notifications-setup-start | ||
| [params] | ||
| (rf/dispatch [:onboarding/notifications-setup-start params])) | ||
|
|
||
| (defn enable-notifications-simple | ||
| (defn notifications-info-view | ||
| [{:keys [blur?]}] | ||
| [quo/documentation-drawers | ||
| {:title (i18n/label :t/enable-notifications) | ||
| :show-button? true | ||
| :shell? blur? | ||
| :button-label (i18n/label :t/read-more) | ||
| :button-icon :i/info} | ||
| [quo/text (i18n/label :t/enable-notifications-info-description)]]) | ||
|
Comment on lines
+28
to
+36
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This drawer view still needs the correct verbiage from the docs team (I think ?) |
||
|
|
||
| (defn on-open-info | ||
| [{:keys [blur? theme] | ||
| :or {blur? true}}] | ||
| (rf/dispatch [:show-bottom-sheet | ||
| {:content (fn [] | ||
| [notifications-info-view {:blur? blur?}]) | ||
| :theme theme | ||
| :shell? blur?}])) | ||
|
|
||
| (defn enable-notification-form | ||
| [{:keys [insets params]}] | ||
| (let [profile-color (rf/sub [:onboarding/customization-color | ||
| {:onboarding? (:onboarding? params)}]) | ||
| [third-party-checked? | ||
| set-third-party-checked] (rn/use-state | ||
| (boolean? (ff/enabled? ::ff/settings.news-notifications))) | ||
| on-enable-notifications (rn/use-callback | ||
| (fn [] | ||
| (on-notifications-setup-start | ||
| (assoc params | ||
| :enable-notifications? true | ||
| :enable-news-notifications? third-party-checked?))) | ||
| [params third-party-checked?]) | ||
| on-skip-notifications (rn/use-callback | ||
| (fn [] | ||
| (on-notifications-setup-start | ||
| (assoc params | ||
| :enable-notifications? false | ||
| :enable-news-notifications? false))) | ||
| [params])] | ||
| [rn/view | ||
| (when (and platform/android? | ||
| (ff/enabled? ::ff/settings.news-notifications)) | ||
| [rn/view | ||
| {:style style/news-notifications-checkbox-container} | ||
| [quo/selectors | ||
| {:type :checkbox | ||
| :blur? true | ||
| :customization-color profile-color | ||
| :checked? third-party-checked? | ||
| :on-change set-third-party-checked}] | ||
| [quo/text | ||
| {:size :paragraph-2 | ||
| :style style/news-notifications-checkbox-text} | ||
| (i18n/label :t/enable-news-notifications-third-party)]]) | ||
| [rn/view {:style (style/buttons insets)} | ||
| [quo/button | ||
| {:on-press on-enable-notifications | ||
| :type :primary | ||
| :icon-left :i/notifications | ||
| :accessibility-label :enable-notifications-button | ||
| :customization-color profile-color} | ||
| (i18n/label :t/intro-wizard-title6)] | ||
| [quo/button | ||
| {:on-press on-skip-notifications | ||
| :accessibility-label :enable-notifications-later-button | ||
| :type :grey | ||
| :background :blur | ||
| :container-style {:margin-top 12}} | ||
| (i18n/label :t/maybe-later)]]])) | ||
|
|
||
| (defn enable-notifications-illustration | ||
| [] | ||
| (let [width (:width (rn/get-window))] | ||
| [rn/image | ||
| {:resize-mode :contain | ||
| :style (style/page-illustration width) | ||
| :source (resources/get-image :notifications)}])) | ||
|
|
||
| (defn background-image | ||
| [] | ||
| [rn/view {:style rn/stylesheet-absolute-fill} | ||
| [background/view true]]) | ||
|
|
||
| (defn view | ||
| [] | ||
| (let [insets safe-area/insets] | ||
| [rn/view {:style (style/page-container insets)} | ||
| [rn/view {:style style/page-heading} | ||
| [quo/page-nav {:type :no-title :background :blur}] | ||
| [page-title]] | ||
| [enable-notifications-simple] | ||
| [enable-notification-buttons {:insets insets}]])) | ||
| (let [insets safe-area/insets | ||
| params (quo.context/use-screen-params)] | ||
| [:<> | ||
| (when-not (:onboarding? params) | ||
| [background-image]) | ||
| [rn/view {:style (style/page-container insets)} | ||
| [rn/view {:style style/page-heading} | ||
| [quo/page-nav | ||
| {:type :no-title | ||
| :background :blur | ||
| :right-side [{:icon-name :i/info | ||
| :on-press on-open-info | ||
| :accessibility-label :notifications-info-button}]}] | ||
| [page-title]] | ||
| [enable-notifications-illustration] | ||
| [enable-notification-form | ||
| {:insets insets | ||
| :params params}]]])) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In release builds it might be better to avoid certain defaults. The idea is that it's better to fail early on somehow rather than to build with incorrect parameters. Ideally, the release build script should fail if required environment vars are missing, but I don't know if we have this capability implemented. My feedback is not a big deal, just a nice to have.