diff --git a/doc/README.md b/doc/README.md index 637ce86026d..301629677be 100644 --- a/doc/README.md +++ b/doc/README.md @@ -10,6 +10,8 @@ [Coding guidelines](new-guidelines.md) +[UI components coding guidelines](ui-guidelines.md) + [Release Checklist](release-checklist.md) [Release Guide](release-guide.md) diff --git a/doc/react_tree.png b/doc/react_tree.png new file mode 100644 index 00000000000..e5ade680dd4 Binary files /dev/null and b/doc/react_tree.png differ diff --git a/doc/ui-guidelines.md b/doc/ui-guidelines.md new file mode 100644 index 00000000000..55aaf4f861a --- /dev/null +++ b/doc/ui-guidelines.md @@ -0,0 +1,254 @@ +# UI components coding guidelines + +> [!IMPORTANT] +> React apps are made out of components. A component is a piece of the UI (user interface) that has its own logic and appearance. A component can be as small as a button, or as large as an entire screen. +> React components are JavaScript functions that return markup +> This document will provide best practices on how to write efficient React components in ClojureScript + + +At the time of creating the Status app, the Reagent library was a solid choice. Back then, hooks didn't exist, and there weren't any libraries providing effective global state management. After Reagent's emergence, another library called Re-frame built upon Reagent. Together, they offered powerful tools for developing React applications with ClojureScript. However, as React evolved, significant changes occurred. Class components, utilized in Reagent, became deprecated. Instead, functional components and hooks emerged for state management. In Status 2.0, we began incorporating more functional components and hooks, resulting in a blend of both approaches. To simplify matters and reduce confusion, we opted to transition to functional components and hooks for local state management. + +BEFORE: +```clojure +(defn- view-internal + [_ _] + (let [pressed? (reagent/atom false)] + (fn + [{:keys [theme on-press on-long-press icon]}] + [rn/pressable + {:style (style/main @pressed? theme) + :on-press on-press + :on-press-in #(reset! pressed? true) + :on-press-out #(reset! pressed? nil) + :on-long-press on-long-press} + [quo.icons/icon icon]]))) + +(def view (theme/with-theme view-internal)) +``` + +NOW: +```clojure +(defn view + [{:keys [on-press on-long-press icon]}] + (let [[pressed? set-pressed] (rn/use-state false) + theme (theme/use-theme-value) + on-press-in (rn/use-callback #(set-pressed true)) + on-press-out (rn/use-callback #(set-pressed nil))] + [rn/pressable + {:style (style/main pressed? theme) + :on-press on-press + :on-press-in on-press-in + :on-press-out on-press-out + :on-long-press on-long-press} + [quo.icons/icon icon]])) +``` + + +- We no longer need to create an anonymous function for rendering. This removes unnecessary confusion and the need for specific knowledge on how it works and why it was needed. +- `rn/use-state` is used instead of `reagent/atom` +- State values no longer need to be dereferenced; they are accessible as regular symbols. This eliminates a common bug where the "@" symbol was inadvertently omitted. +- `theme/with-theme` wrapper is not needed anymore, `(theme/use-theme-value)` hook can be used directly in the components +- `:f>` not needed anymore, all components are functional by default +- `rn/use-callback` hook should be used for anon callback functions + +> [!IMPORTANT] +> DO NOT USE anon functions directly in the props + +BAD +```clojure +(defn view + [] + (let [[pressed? set-pressed] (rn/use-state false)] + [rn/pressable + {:style (style/main pressed?) + :on-press-in #(set-pressed true) + :on-press-out #(set-pressed nil)}])) +``` + +GOOD: +```clojure +(defn view + [] + (let [[pressed? set-pressed] (rn/use-state false) + on-press-in (rn/use-callback #(set-pressed true)) + on-press-out (rn/use-callback #(set-pressed nil))] + [rn/pressable + {:style (style/main pressed?) + :on-press-in on-press-in + :on-press-out on-press-out}])) +``` + +## Global state and subscriptions + +For global state management, we utilize Re-frame subscriptions. They can be likened to React state. To obtain the state, `(rf/sub [])` is employed, and to modify it, `(rf/dispatch [])` is utilized. However, they update components in a similar manner to React states. + +```clojure +(defn view + [{:keys [selected-tab]}] + (let [collectible-list (rf/sub [:wallet/all-collectibles]) + on-collectible-press (rn/use-callback + (fn [{:keys [id]}] + (rf/dispatch [:wallet/get-collectible-details id])))] + [rn/view {:style style/container} + (case selected-tab + :assets [assets/view] + :collectibles [collectibles/view {:collectibles collectible-list + :on-collectible-press on-collectible-press}]) + [activity/view]])) +``` + +## Regular atoms + +In certain instances, components utilized regular atoms; however, they should now be used with `rn/use-ref-atom` + +BEFORE: +```clojure +(defn view + [] + (let [focused? (atom false)] + (fn [] + (let [on-clear #(reset! status (if @focused? :active :default)) + on-focus #(reset! focused? true) + on-blur #(reset! focused? false)])))) +``` + +NOW: +```clojure +(defn view + [] + (let [focused? (rn/use-ref-atom false) + on-clear (rn/use-callback #(set-status (if @focused? :active :default))) + on-focus (rn/use-callback #(reset! focused? true)) + on-blur (rn/use-callback #(reset! focused? false))])) +``` + +## Effects + +LIFECYCLE: + +```clojure +(defn view + [{:keys []}] + (let [opacity (reanimated/use-shared-value 0)] + (rn/use-mount #(reanimated/animate opacity 1)) + [rn/view + {:style (style/opacity opacity)}])) +``` + +```clojure +(defn view + [{:keys []}] + (let [] + (rn/use-unmount #(rn/dispatch [:unmounted])) + [rn/view])) +``` + +> [!IMPORTANT] +> Effects should NOT be utilized as a response to state changes for modifying logic. If you're unsure how to achieve this without using effects, please consult the team in the general chat. There may be instances where using effects is appropriate, so we can explore a solution together and enhance our guidelines. + +BAD: +```clojure +(defn f-zoom-button + [{:keys [selected? current-zoom]}] + (let [size (reanimated/use-shared-value (if selected? 37 25))] + (rn/use-effect #(reanimated/animate size (if selected? 37 25)) [current-zoom]) + [rn/touchable-opacity + {:style (style/zoom-button-container size)}])) +``` + +BAD: + +```clojure +(defn view + [collectible-list (rf/sub [:wallet/all-collectibles])] + (let [] + (rn/use-effect #(rn/dispatch [:all-collectibles-changed]) [collectible-list]) + [rn/view])) +``` + +Instead `:all-collectibles-changed` should be used in the handler which changes `collectible-list` state + + + +## Performance tips + +To begin with, we need to understand that there are two distinct stages for a component: creation and update. React creates a render tree, a UI tree, composed of the rendered components. + +![react_tree.png](react_tree.png) + +### Component creation + +For component creation, the most critical factor is the number of elements involved, so we should strive to minimize them. For instance, it's advisable to avoid using unnecessary wrappers or containers. + +BAD: + +```clojure +(defn view + [] + (let [] + [rn/view {:style {:padding-top 20}} + [quo/button]])) +``` + +GOOD: +```clojure +(defn view + [] + (let [] + [quo/button {:container-style {:padding-top 20}}])) +``` + +### Component updates + +For component updates, it's crucial to recognize that React will invoke the function where state is utilized. Therefore, if you utilize state in the root component, React will execute the root function and re-render the entire root component along with all its children (unless optimizations like memoization are implemented). + +BAD: + +```clojure +(defn component + [{:keys [label]}] + (let [] + [rn/text label])) + +(defn component2 + [{:keys [label2]}] + (let [] + [rn/text label2])) + +(defn screen + [] + (let [screen-params (rf/sub [:screen-params])] + [component screen-params] + [component1] + [component2 screen-params] + [component3] + [rn/view {:padding-top 20} + [quo/button]])) +``` + +Here, we have lost control over the `screen-params` map. It can contain any data, and if any field within this map changes, the entire screen function will be executed, resulting in the re-rendering of both `component` and `component2`. + +GOOD: +```clojure +(defn component + [] + (let [label (rf/sub [:screen-params-label])] + [rn/text label])) + +(defn component2 + [] + (let [label2 (rf/sub [:screen-params-label2])] + [rn/text label2])) + +(defn screen + [] + (let [] + [component] + [component1] + [component2] + [component3] + [rn/view {:padding-top 20} + [quo/button]])) +``` + +So, now the screen component function will never be invoked, and `component` and `component2` will be re-rendered only when `label` or `label2` have changed. diff --git a/src/legacy/status_im/bottom_sheet/sheets.cljs b/src/legacy/status_im/bottom_sheet/sheets.cljs index 1f531fb5258..537d4117b05 100644 --- a/src/legacy/status_im/bottom_sheet/sheets.cljs +++ b/src/legacy/status_im/bottom_sheet/sheets.cljs @@ -28,10 +28,10 @@ [:f> (fn [] - (rn/use-effect (fn [] - (rn/hw-back-add-listener dismiss-bottom-sheet-callback) - (fn [] - (rn/hw-back-remove-listener dismiss-bottom-sheet-callback)))) + (rn/use-mount (fn [] + (rn/hw-back-add-listener dismiss-bottom-sheet-callback) + (fn [] + (rn/hw-back-remove-listener dismiss-bottom-sheet-callback)))) [theme/provider {:theme (or page-theme (theme/get-theme))} [bottom-sheet/bottom-sheet opts (when content diff --git a/src/quo/components/colors/color_picker/view.cljs b/src/quo/components/colors/color_picker/view.cljs index 14579fc044e..84c9445f034 100644 --- a/src/quo/components/colors/color_picker/view.cljs +++ b/src/quo/components/colors/color_picker/view.cljs @@ -27,7 +27,7 @@ (let [selected (reagent/atom default-selected) {window-width :width} (rn/get-window) ref (atom nil)] - (rn/use-effect + (rn/use-mount (fn [] (js/setTimeout (fn [] diff --git a/src/quo/components/drawers/drawer_buttons/view.cljs b/src/quo/components/drawers/drawer_buttons/view.cljs index f5fc9301172..c496bf21df8 100644 --- a/src/quo/components/drawers/drawer_buttons/view.cljs +++ b/src/quo/components/drawers/drawer_buttons/view.cljs @@ -177,9 +177,9 @@ 1 animations-duration :easing4))] - (rn/use-effect (fn [] - (when on-init - (on-init reset-top-animation)))) + (rn/use-mount (fn [] + (when on-init + (on-init reset-top-animation)))) [reanimated/view {:style (style/outer-container height border-radius container-style)} [blur/view {:blur-type :dark diff --git a/src/quo/components/record_audio/record_audio/view.cljs b/src/quo/components/record_audio/record_audio/view.cljs index e7cff3b571d..53371b62427 100644 --- a/src/quo/components/record_audio/record_audio/view.cljs +++ b/src/quo/components/record_audio/record_audio/view.cljs @@ -15,7 +15,7 @@ [quo.foundations.colors :as colors] [quo.theme :as quo.theme] [react-native.audio-toolkit :as audio] - [react-native.core :as rn :refer [use-effect]] + [react-native.core :as rn] [react-native.platform :as platform] [reagent.core :as reagent] [taoensso.timbre :as log] @@ -528,20 +528,20 @@ (reset! reached-max-duration? false)) (reset! touch-timestamp nil))] (fn [] - (use-effect (fn [] - (when on-check-audio-permissions - (on-check-audio-permissions)) - (when on-init - (on-init reset-recorder)) - (when audio-file - (let [filename (last (string/split audio-file "/"))] - (reload-player filename))) - (reset! app-state-listener - (.addEventListener rn/app-state - "change" - #(when (= % "background") - (reset! playing-audio? false)))) - #(.remove @app-state-listener))) + (rn/use-mount (fn [] + (when on-check-audio-permissions + (on-check-audio-permissions)) + (when on-init + (on-init reset-recorder)) + (when audio-file + (let [filename (last (string/split audio-file "/"))] + (reload-player filename))) + (reset! app-state-listener + (.addEventListener rn/app-state + "change" + #(when (= % "background") + (reset! playing-audio? false)))) + #(.remove @app-state-listener))) [rn/view {:style style/bar-container :pointer-events :box-none} diff --git a/src/react_native/core.cljs b/src/react_native/core.cljs index 6b316189ad0..4f62cf66580 100644 --- a/src/react_native/core.cljs +++ b/src/react_native/core.cljs @@ -2,7 +2,6 @@ (:require ["react" :as react] ["react-native" :as react-native] - [cljs-bean.core :as bean] [oops.core :as oops] [react-native.flat-list :as flat-list] [react-native.platform :as platform] @@ -135,26 +134,50 @@ (def use-context react/useContext) +(defn use-ref-atom + [value] + (let [ref (use-ref (atom value))] + (.-current ^js ref))) + +(defn get-js-deps + [deps] + (if deps + (let [prev-state (use-ref-atom {:value false :deps nil}) + prev-deps (:deps @prev-state) + prev-value (:value @prev-state)] + (if (and (not (nil? prev-deps)) (not= (count deps) (count prev-deps))) + (throw (js/Error. "Hooks can't have a different number of dependencies across re-renders")) + (if (not= deps prev-deps) + (let [new-value (not prev-value)] + (reset! prev-state {:value new-value + :deps deps}) + #js [new-value]) + #js [prev-value]))) + js/undefined)) + (defn use-effect - ([effect-fn] - (use-effect effect-fn [])) - ([effect-fn deps] + {:deprecated + "use-mount or use-unmount should be used, more here https://github.com/status-im/status-mobile/blob/develop/doc/ui-guidelines.md#effects"} + ([handler] + (use-effect handler nil)) + ([handler deps] (react/useEffect - #(let [ret (effect-fn)] - (if (fn? ret) ret js/undefined)) - (bean/->js deps)))) - -(def use-callback react/useCallback) + #(let [ret (handler)] (if (fn? ret) ret js/undefined)) + (get-js-deps deps)))) -(defn use-effect-once - [effect-fn] - (use-effect effect-fn)) +(defn use-mount + [handler] + (use-effect handler [])) (defn use-unmount - [f] - (let [fn-ref (use-ref f)] - (oops/oset! fn-ref "current" f) - (use-effect-once (fn [] (fn [] (oops/ocall! fn-ref "current")))))) + [handler] + (use-mount (fn [] handler))) + +(defn use-callback + ([handler] + (use-callback handler [])) + ([handler deps] + (react/useCallback handler (get-js-deps deps)))) (def layout-animation (.-LayoutAnimation ^js react-native)) (def configure-next (.-configureNext ^js layout-animation)) diff --git a/src/status_im/common/bottom_sheet_screen/view.cljs b/src/status_im/common/bottom_sheet_screen/view.cljs index e9783b456e6..40a65565cf0 100644 --- a/src/status_im/common/bottom_sheet_screen/view.cljs +++ b/src/status_im/common/bottom_sheet_screen/view.cljs @@ -64,7 +64,7 @@ (reanimated/animate opacity 1 300) (set-animating-false 300) (reset! scroll-enabled? true))] - (rn/use-effect + (rn/use-mount (fn [] (rn/hw-back-add-listener close) (reanimated/animate translate-y 0 300) diff --git a/src/status_im/common/lightbox/utils.cljs b/src/status_im/common/lightbox/utils.cljs index 2f6e71b7115..b1d34ab575e 100644 --- a/src/status_im/common/lightbox/utils.cljs +++ b/src/status_im/common/lightbox/utils.cljs @@ -27,7 +27,7 @@ (defn effect [{:keys [flat-list-ref scroll-index-lock? timers]} {:keys [opacity layout border]} index] - (rn/use-effect + (rn/use-mount (fn [] (reagent/next-tick (fn [] (when @flat-list-ref diff --git a/src/status_im/common/lightbox/zoomable_image/view.cljs b/src/status_im/common/lightbox/zoomable_image/view.cljs index a82e4f015cc..7f777114c72 100644 --- a/src/status_im/common/lightbox/zoomable_image/view.cljs +++ b/src/status_im/common/lightbox/zoomable_image/view.cljs @@ -259,8 +259,8 @@ dimensions animations state))] - (rn/use-effect (fn [] - (js/setTimeout (fn [] (reset! set-full-height? true)) 500))) + (rn/use-mount (fn [] + (js/setTimeout (fn [] (reset! set-full-height? true)) 500))) (when platform/ios? (utils/handle-orientation-change curr-orientation focused? dimensions animations state) (utils/handle-exit-lightbox-signal exit-lightbox-signal diff --git a/src/status_im/common/scan_qr_code/view.cljs b/src/status_im/common/scan_qr_code/view.cljs index 5ee2734c23e..80a72e80c92 100644 --- a/src/status_im/common/scan_qr_code/view.cljs +++ b/src/status_im/common/scan_qr_code/view.cljs @@ -202,7 +202,7 @@ (boolean (not-empty @qr-view-finder))) camera-ready-to-scan? (and show-camera? (not @qr-code-succeed?))] - (rn/use-effect + (rn/use-mount (fn [] (rn/hw-back-add-listener navigate-back-handler) (set-listener-torch-off-on-app-inactive torch?) diff --git a/src/status_im/contexts/chat/messenger/composer/effects.cljs b/src/status_im/contexts/chat/messenger/composer/effects.cljs index 02e0ff24011..cd112f55278 100644 --- a/src/status_im/contexts/chat/messenger/composer/effects.cljs +++ b/src/status_im/contexts/chat/messenger/composer/effects.cljs @@ -222,7 +222,7 @@ (defn did-mount [{:keys [selectable-input-ref input-ref selection-manager]}] - (rn/use-effect + (rn/use-mount (fn [] (when platform/android? (let [selectable-text-input-handle (rn/find-node-handle @selectable-input-ref) diff --git a/src/status_im/contexts/chat/messenger/messages/content/audio/view.cljs b/src/status_im/contexts/chat/messenger/messages/content/audio/view.cljs index c829a929d4c..d31dfc92dd1 100644 --- a/src/status_im/contexts/chat/messenger/messages/content/audio/view.cljs +++ b/src/status_im/contexts/chat/messenger/messages/content/audio/view.cljs @@ -159,7 +159,7 @@ paused? (= (audio/get-state player) audio/PAUSED) app-state (rf/sub [:app-state]) mediaserver-port (rf/sub [:mediaserver/port])] - (rn/use-effect (fn [] #(destroy-player player-key))) + (rn/use-mount (fn [] #(destroy-player player-key))) (rn/use-effect (fn [] (when (or diff --git a/src/status_im/contexts/onboarding/common/overlay/view.cljs b/src/status_im/contexts/onboarding/common/overlay/view.cljs index 3ae44ead9a1..61e0cdfd807 100644 --- a/src/status_im/contexts/onboarding/common/overlay/view.cljs +++ b/src/status_im/contexts/onboarding/common/overlay/view.cljs @@ -55,7 +55,7 @@ (let [opacity (reanimated/use-shared-value (if (zero? @blur-amount) 0 1)) blur-show-fn #(blur-show opacity blur-amount) blur-dismiss-fn #(blur-dismiss opacity blur-amount)] - (rn/use-effect + (rn/use-mount (fn [] (reset! blur-show-fn-atom blur-show-fn) (reset! blur-dismiss-fn-atom blur-dismiss-fn) diff --git a/src/status_im/contexts/onboarding/identifiers/view.cljs b/src/status_im/contexts/onboarding/identifiers/view.cljs index 9c7da076898..e2834cbc491 100644 --- a/src/status_im/contexts/onboarding/identifiers/view.cljs +++ b/src/status_im/contexts/onboarding/identifiers/view.cljs @@ -34,10 +34,7 @@ photo-path (rf/sub [:chats/photo-path public-key]) emoji-string (string/join emoji-hash)] (carousel.animation/use-initialize-animation progress paused? true is-dragging? drag-amount) - (rn/use-effect - (fn [] - (carousel.animation/cleanup-animation progress paused?)) - []) + (rn/use-mount #(carousel.animation/cleanup-animation progress paused?)) [:<> [rn/view {:style style/page-container} [carousel/view diff --git a/src/status_im/contexts/profile/profiles/view.cljs b/src/status_im/contexts/profile/profiles/view.cljs index b6aa3a39fba..16fb76d53fc 100644 --- a/src/status_im/contexts/profile/profiles/view.cljs +++ b/src/status_im/contexts/profile/profiles/view.cljs @@ -137,12 +137,12 @@ [{:keys [set-hide-profiles]}] (let [profiles (vals (rf/sub [:profile/profiles-overview])) translate-x (reanimated/use-shared-value @translate-x-atom)] - (rn/use-effect (fn [] - (reset! push-animation-fn-atom #(push-animation translate-x)) - (reset! pop-animation-fn-atom #(pop-animation translate-x)) - (fn [] - (reset! push-animation-fn-atom nil) - (reset! pop-animation-fn-atom nil)))) + (rn/use-mount (fn [] + (reset! push-animation-fn-atom #(push-animation translate-x)) + (reset! pop-animation-fn-atom #(pop-animation translate-x)) + (fn [] + (reset! push-animation-fn-atom nil) + (reset! pop-animation-fn-atom nil)))) [reanimated/view {:style (style/profiles-container translate-x)} [rn/view diff --git a/src/status_im/contexts/shell/jump_to/view.cljs b/src/status_im/contexts/shell/jump_to/view.cljs index b64041ad09e..fce1e5e87b2 100644 --- a/src/status_im/contexts/shell/jump_to/view.cljs +++ b/src/status_im/contexts/shell/jump_to/view.cljs @@ -33,11 +33,10 @@ (defn f-shell-stack [] (let [shared-values (shared-values/calculate-and-set-shared-values)] - (rn/use-effect + (rn/use-mount (fn [] (rn/hw-back-add-listener navigate-back-handler) - #(rn/hw-back-remove-listener navigate-back-handler)) - []) + #(rn/hw-back-remove-listener navigate-back-handler))) [:<> [jump-to-screen/view] [:f> bottom-tabs/f-bottom-tabs] diff --git a/src/status_im/contexts/syncing/scan_sync_code/view.cljs b/src/status_im/contexts/syncing/scan_sync_code/view.cljs index a6fc5a5c364..19b3f7d4ef9 100644 --- a/src/status_im/contexts/syncing/scan_sync_code/view.cljs +++ b/src/status_im/contexts/syncing/scan_sync_code/view.cljs @@ -321,8 +321,7 @@ :subtitle-opacity subtitle-opacity :title-opacity title-opacity})] - (rn/use-effect - #(set-listener-torch-off-on-app-inactive torch?)) + (rn/use-mount #(set-listener-torch-off-on-app-inactive torch?)) (when animated? (rn/use-effect @@ -335,7 +334,7 @@ (animation/animate-title title-opacity) (animation/animate-bottom bottom-view-translate-y)) - (rn/use-effect + (rn/use-mount (fn initialize-component [] (when animated? (animation/animate-content content-opacity) diff --git a/src/status_im/contexts/wallet/collectible/view.cljs b/src/status_im/contexts/wallet/collectible/view.cljs index 44a76f2cfcc..aa967222d6e 100644 --- a/src/status_im/contexts/wallet/collectible/view.cljs +++ b/src/status_im/contexts/wallet/collectible/view.cljs @@ -116,10 +116,7 @@ {collection-image :image-url collection-name :name} collection-data {collectible-name :name} collectible-data] - (rn/use-effect - (fn [] - #(rf/dispatch [:wallet/clear-last-collectible-details])) - []) + (rn/use-unmount #(rf/dispatch [:wallet/clear-last-collectible-details])) [scroll-page/scroll-page {:navigate-back? true :height 148 diff --git a/src/status_im/contexts/wallet/send/input_amount/view.cljs b/src/status_im/contexts/wallet/send/input_amount/view.cljs index ffcd9e2a8ad..b5de6419f2a 100644 --- a/src/status_im/contexts/wallet/send/input_amount/view.cljs +++ b/src/status_im/contexts/wallet/send/input_amount/view.cljs @@ -239,7 +239,7 @@ (utils/get-standard-fiat-format fee-in-crypto-formatted currency-symbol fee-in-fiat))] - (rn/use-effect + (rn/use-mount (fn [] (let [dismiss-keyboard-fn #(when (= % "active") (rn/dismiss-keyboard!)) app-keyboard-listener (.addEventListener rn/app-state "change" dismiss-keyboard-fn)]