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

fix device theme change listener in ios #15724

Merged
merged 2 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/react_native/navigation.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
[comp]
(.catch (.dismissOverlay Navigation comp) #()))

(defn dissmiss-all-overlays
[]
(.catch (.dismissAllOverlays Navigation) #()))

(defn reg-app-launched-listener
[handler]
(.registerAppLaunchedListener ^js (.events ^js Navigation) handler))
Expand Down
7 changes: 7 additions & 0 deletions src/status_im/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@
status-im2.contexts.onboarding.events
status-im.chat.models.gaps
[status-im2.navigation.events :as navigation]
[status-im2.common.theme.core :as theme]
[react-native.core :as rn]
[react-native.platform :as platform]
status-im2.contexts.chat.home.events))

(re-frame/reg-fx
Expand Down Expand Up @@ -90,6 +93,10 @@
(re-frame/reg-fx
::app-state-change-fx
(fn [state]
(when (and platform/ios? (= state "active"))
;; Change the app theme if the ios device theme was updated when the app was in the background
;; https://github.com/status-im/status-mobile/issues/15708
(theme/change-device-theme (rn/get-color-scheme)))
(status/app-state-change state)))

(re-frame/reg-fx
Expand Down
1 change: 1 addition & 0 deletions src/status_im/multiaccounts/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@
(re-frame/dispatch [:change-shell-status-bar-style
(if (shell.animation/home-stack-open?) status-bar-theme :light)])
(when reload-ui?
(rf/dispatch [:dissmiss-all-overlays])
(hot-reload/reload)
(when-not (= view-id :shell-stack)
(re-frame/dispatch [:change-shell-nav-bar-color nav-bar-color]))))))
Expand Down
27 changes: 19 additions & 8 deletions src/status_im2/common/theme/core.cljs
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
(ns status-im2.common.theme.core
(:require [quo.theme :as quo]
[quo2.theme :as quo2]
[react-native.core :as rn]))
[utils.re-frame :as rf]
[oops.core :refer [oget]]
[react-native.core :as rn]
[react-native.platform :as platform]))

(def device-theme (atom (rn/get-color-scheme)))

;; Note - don't use value returned by change listener
;; https://github.com/facebook/react-native/issues/28525
(defn change-device-theme
[theme]
(when-not (= theme @device-theme)
(reset! device-theme theme)
(rf/dispatch [:system-theme-mode-changed (keyword theme)])))

;; Appearance change listener fires false events in ios when the app is in the background
;; So, we are ignoring those events and when the device returns form the background,
;; we are manually checking the device theme, in ::app-state-change-fx
;; https://github.com/status-im/status-mobile/issues/15708
(defn add-device-theme-change-listener
[callback]
(rn/appearance-add-change-listener #(let [theme (rn/get-color-scheme)]
(when-not (= theme @device-theme)
(reset! device-theme theme)
(callback (keyword theme))))))
[]
(rn/appearance-add-change-listener
#(when (or platform/android?
(not= (oget rn/app-state "currentState") "background"))
(change-device-theme (oget % "colorScheme")))))

(defn device-theme-dark?
[]
Expand Down
3 changes: 1 addition & 2 deletions src/status_im2/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
(re-frame/reg-fx
:setup/init-theme
(fn []
(theme/add-device-theme-change-listener
#(re-frame/dispatch [:system-theme-mode-changed %]))))
(theme/add-device-theme-change-listener)))

(rf/defn initialize-views
{:events [:setup/initialize-view]}
Expand Down
4 changes: 4 additions & 0 deletions src/status_im2/navigation/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@
;; OVERLAY
(def dissmiss-overlay navigation/dissmiss-overlay)

(def dissmiss-all-overlays navigation/dissmiss-all-overlays)

(defn show-overlay
([comp] (show-overlay comp {}))
([comp opts]
Expand All @@ -173,6 +175,8 @@
:overlay {:interceptTouchOutside true}}
opts)}})))

(re-frame/reg-fx :dissmiss-all-overlays-fx dissmiss-all-overlays)

;; toast
(navigation/register-component "toasts" (fn [] views/toasts) js/undefined)

Expand Down
10 changes: 9 additions & 1 deletion src/status_im2/navigation/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
{:events [:hide-bottom-sheet]}
[{:keys [db]}]
(let [{:keys [hide? sheets]} (:bottom-sheet db)]
(println :hide-bottom-sheet (not hide?) (seq sheets))
(when (and (not hide?) (seq sheets))
{:db (assoc-in db [:bottom-sheet :hide?] true)})))

Expand Down Expand Up @@ -140,3 +139,12 @@
key-uid
:keycard-pairing]))]
{:set-root (if keycard-account? :multiaccounts-keycard :multiaccounts)}))

(rf/defn dismiss-all-overlays
{:events [:dissmiss-all-overlays]}
[{:keys [db]}]
{:dissmiss-all-overlays-fx nil
:db (-> db
(dissoc :popover/popover)
Copy link
Member

Choose a reason for hiding this comment

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

what about bottom sheet ?

Copy link
Member Author

@Parveshdhull Parveshdhull Apr 26, 2023

Choose a reason for hiding this comment

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

hi @flexsurfer, Thank you for the reviewing PR.

I just skipped that one, because for all overlays (navigation/dissmiss-all-overlays) works/enough, except for above two. For these two we also have to remove them from db. (not sure why)

Copy link
Member Author

Choose a reason for hiding this comment

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

added now

(dissoc :visibility-status-popover/popover)
(assoc-in [:bottom-sheet :hide?] true))})