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

Share wallet address [17865] #18159

Closed
wants to merge 1 commit into from
Closed

Conversation

tumanov-alex
Copy link
Contributor

@tumanov-alex tumanov-alex commented Dec 12, 2023

fixes #17865

Share wallet address design update (UI only)

While I'm working on adding share-qr-code to the wallet share screen and re-using swipeable status component, I've decided to share the UI part asap

Steps to test

  • go to quo library
  • scroll all the way down to share-qr-code

Before and after screenshots comparison

Before:

After:

| Figma (if available)

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Dec 12, 2023

Jenkins Builds

Click to see older builds (7)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ e283aaf #1 2023-12-12 14:09:44 ~7 min android-e2e 🤖apk 📲
✔️ e283aaf #1 2023-12-12 14:10:17 ~8 min android 🤖apk 📲
✔️ e283aaf #1 2023-12-12 14:14:17 ~12 min ios 📱ipa 📲
d6c0623 #2 2023-12-12 19:36:16 ~5 min tests 📄log
✔️ d6c0623 #2 2023-12-12 19:37:33 ~6 min ios 📱ipa 📲
✔️ d6c0623 #2 2023-12-12 19:37:57 ~6 min android 🤖apk 📲
✔️ d6c0623 #2 2023-12-12 19:38:58 ~8 min android-e2e 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
d5c0eec #3 2023-12-12 21:30:51 ~4 min tests 📄log
✔️ d5c0eec #3 2023-12-12 21:34:22 ~8 min android-e2e 🤖apk 📲
✔️ d5c0eec #3 2023-12-12 21:34:29 ~8 min android 🤖apk 📲
✔️ d5c0eec #3 2023-12-12 21:36:47 ~10 min ios 📱ipa 📲
✔️ 2f2ad8a #4 2023-12-12 21:42:09 ~4 min tests 📄log
✔️ 2f2ad8a #4 2023-12-12 21:44:10 ~6 min android 🤖apk 📲
✔️ 2f2ad8a #4 2023-12-12 21:45:18 ~7 min android-e2e 🤖apk 📲
✔️ 2f2ad8a #4 2023-12-12 21:47:59 ~10 min ios 📱ipa 📲

Copy link
Contributor

@ulisesmac ulisesmac left a comment

Choose a reason for hiding this comment

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

Please make sure to update the docstring and component API to properly reflect the new design.

Also, please update the preview screen and test if needed.

In PR's description, the screenshot of the new design doesn't show the configuration icon, so make sure it's being properly rendered.

Comment on lines 91 to 92
(def ^:private dashed-line-width 2)
(def ^:private dashed-line-space 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are not going to use these styles anymore, we should remove them 👍

Comment on lines 46 to 47
:style
(style/header-title component-width)
Copy link
Contributor

Choose a reason for hiding this comment

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

They should be in the same line (46)

Comment on lines -10 to +16
$ yarn shadow-cljs run re-frisk-remote.core/start
yarn shadow-cljs run re-frisk-remote.core/start
```

or you can also use make:

```bash
$ make run-re-frisk
make run-re-frisk
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe unrelated changes?

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 know that i should create a separate pr for these two lines, but since can we smuggle them here? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

do we have an isue for it or something?

@tumanov-alex tumanov-alex force-pushed the 17865-share-wallet-address branch 2 times, most recently from d6c0623 to d5c0eec Compare December 12, 2023 21:25
@tumanov-alex tumanov-alex force-pushed the 17865-share-wallet-address branch from d5c0eec to 2f2ad8a Compare December 12, 2023 21:37
@ulisesmac
Copy link
Contributor

Hey Alex, by loking at the screenshots seems the grandient is also missing 😅

@J-Son89 J-Son89 closed this Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

Share Screen -> Implement Share Wallet Address
5 participants