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.

Sign up for GitHub

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

[#21198] Sync error button covered #21454

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

[#21198] Sync error button covered #21454

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e9833e
WIP: fix spacing and code style
ulisesmac Oct 17, 2024
d6041c9
WIP
ulisesmac Oct 18, 2024
7ffa680
set fixed bottom as designs
ulisesmac Oct 18, 2024
b6ab9ca
lint fix
ulisesmac Oct 18, 2024
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: 3 additions & 1 deletion src/status_im/contexts/onboarding/syncing/progress/style.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns status-im.contexts.onboarding.syncing.progress.style
(:require
[quo.foundations.colors :as colors]))
[quo.foundations.colors :as colors]
[react-native.safe-area :as safe-area]))

(def absolute-fill
{:position :absolute
Expand All @@ -17,6 +18,7 @@
:bottom 0
:left 0
:right 0
:padding-top (safe-area/get-top)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the safe area for the top

:padding-bottom 20
:background-color (when-not in-onboarding? colors/neutral-80-opa-80-blur)})

Expand Down
67 changes: 36 additions & 31 deletions src/status_im/contexts/onboarding/syncing/progress/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@

(defn page-title
[pairing-progress?]
[quo/text-combinations
{:container-style {:margin-top 56 :margin-horizontal 20}
:title (i18n/label (if pairing-progress?
:t/sync-devices-title
:t/sync-devices-error-title))
:description (i18n/label (if pairing-progress?
:t/sync-devices-sub-title
:t/sync-devices-error-sub-title))
:title-accessibility-label :progress-screen-title
:description-accessibility-label :progress-screen-sub-title}])
[quo/page-top {:title (i18n/label (if pairing-progress?
:t/sync-devices-title
:t/sync-devices-error-title))
:description :text
:description-text (i18n/label (if pairing-progress?
:t/sync-devices-sub-title
:t/sync-devices-error-sub-title))}])

(defn- navigate-to-enter-seed-phrase
[]
Expand All @@ -42,32 +39,40 @@

(defn try-again-button
[profile-color logged-in?]
[quo/bottom-actions
{:actions (if logged-in? :one-action :two-actions)
:blur? true
:button-one-label (i18n/label :t/recovery-phrase)
:button-one-props {:type :primary
:accessibility-label :try-seed-phrase-button
:customization-color profile-color
:container-style {:flex 1}
:size 40
:on-press navigate-to-enter-seed-phrase}
(if logged-in? :button-one-label :button-two-label)
(i18n/label :t/try-again)
(if logged-in? :button-one-props :button-two-props)
{:type (if logged-in? :primary :grey)
:accessibility-label :try-again-later-button
:customization-color profile-color
:container-style {:flex 1}
:size 40
:on-press #(try-again logged-in?)}}])
Comment on lines -45 to -63
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored this code, but it's very interesting.

@ilmotta FYI, just in case you didn't know, this code has the following structure:

{:foo              1
 :bar              2
 (if true :foo :x) 10
 (if true :bar :y) 20}

so weird, because this is invalid in Clojure, it throws (as expected), an exception due to duplicated keys, but in Clojurescript, it overrides the previously written keys:

;; =>
 {:foo 10
  :bar 20}

as this may not work in a future release (idk) I refactored to be Clojure-valid

Copy link
Contributor

Choose a reason for hiding this comment

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

Strange indeed @ulisesmac. If we see this type of code we can make an effort to refactor if possible because we have other ways, like using cond->.

(let [number-of-actions (if logged-in? :one-action :two-actions)
try-again-label (i18n/label :t/try-again)
try-again-props {:type (if logged-in? :primary :grey)
:accessibility-label :try-again-later-button
:customization-color profile-color
:container-style {:flex 1}
:size 40
:on-press #(try-again logged-in?)}
recovery-phrase-label (i18n/label :t/recovery-phrase)
recovery-phrase-props {:type :primary
:accessibility-label :try-seed-phrase-button
:customization-color profile-color
:container-style {:flex 1}
:size 40
:on-press navigate-to-enter-seed-phrase}
buttons (if logged-in?
{:button-one-label try-again-label
:button-one-props try-again-props}
{:button-one-label recovery-phrase-label
:button-one-props recovery-phrase-props
:button-two-label try-again-label
:button-two-props try-again-props})]
[quo/bottom-actions (assoc buttons
:actions number-of-actions
:blur? true)]))

(defn- illustration
[pairing-progress?]
[rn/image
{:resize-mode :contain
:style (style/page-illustration (:width (rn/get-window)))
:source (resources/get-image (if pairing-progress? :syncing-devices :syncing-wrong))}])
:source (resources/get-image (if pairing-progress?
:syncing-devices
:syncing-wrong))}])

(defn view
[in-onboarding?]
Expand All @@ -83,7 +88,7 @@
[quo/page-nav {:type :no-title :background :blur}]
[page-title pairing-progress?]
[illustration pairing-progress?]
(when-not (pairing-progress pairing-status)
(when-not pairing-progress?
[try-again-button profile-color logged-in?])]))

(defn view-onboarding
Expand Down