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

[#21198] Sync error button covered #21454

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

Conversation

ulisesmac
Copy link
Contributor

fixes #21198

Summary

This PR fixes wrong paddings (safe-area related) in a screen.

Platforms

  • iOS

status: ready

@ulisesmac
Copy link
Contributor Author

Discussion related:
https://www.figma.com/design/oznddnBnDbLlLQIaACYNuj?node-id=2249-7147&m=dev#991338702

CC: @Francesca-G

@@ -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

@status-im-auto
Copy link
Member

status-im-auto commented Oct 18, 2024

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
7ffa680 #1 2024-10-18 05:40:24 ~3 min tests 📄log
✔️ b6ab9ca #2 2024-10-18 05:48:51 ~4 min tests 📄log
✔️ b6ab9ca #2 2024-10-18 05:51:59 ~7 min android-e2e 🤖apk 📲
✔️ b6ab9ca #2 2024-10-18 05:52:05 ~7 min android 🤖apk 📲
✔️ b6ab9ca #2 2024-10-18 05:55:42 ~11 min ios 📱ipa 📲

Comment on lines -45 to -63
[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?)}}])
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->.

Copy link
Member

@Parveshdhull Parveshdhull left a comment

Choose a reason for hiding this comment

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

Thank you @ulisesmac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync error screen vertical spacing prevents from clicking a button
5 participants