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

chore(wallet): feature flag graph and hide about action button #19832

Merged
merged 2 commits into from
May 6, 2024

Conversation

J-Son89
Copy link
Contributor

@J-Son89 J-Son89 commented Apr 29, 2024

In the Offsite it was agreed to feature flag the wallet graphs until a later release.

see the design file:
https://www.figma.com/file/xLs1KYmF4e6WwRTZVJKeUK/Descoped-Wallet?type=design&node-id=7503-243135&mode=design&t=dtWnJDJElRxhMoag-0

This pr hides the home page graph and the account page graph.
It also hides the options button on the account "about" tab.

Home Page:

Account Page:

About Tab:

:height 130
:align-items :center
:justify-content :center})
{:height 110
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to the page using and passed via container-style - (also updated preview screen)

:justify-content :center
:flex 1
:max-height 106})
(def inner-container
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to the page using and passed via container-style - (also updated preview screen)

::community.edit-account-selection (enabled-in-env? :FLAG_EDIT_ACCOUNT_SELECTION_ENABLED)
::wallet.contacts (enabled-in-env? :FLAG_CONTACTS_ENABLED)}))
::wallet.remove-account (enabled-in-env? :FLAG_REMOVE_ACCOUNT_ENABLED)
}))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alpha sorted ^

@J-Son89 J-Son89 marked this pull request as ready for review April 29, 2024 20:51
@status-im-auto
Copy link
Member

status-im-auto commented Apr 29, 2024

Jenkins Builds

Click to see older builds (13)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 91351ac #1 2024-04-29 20:51:32 ~4 min tests 📄log
✔️ 4fa3ea0 #2 2024-04-29 20:55:57 ~4 min tests 📄log
✔️ 4fa3ea0 #2 2024-04-29 20:57:35 ~5 min android-e2e 🤖apk 📲
✔️ 4fa3ea0 #2 2024-04-29 20:59:04 ~7 min android 🤖apk 📲
✔️ 4fa3ea0 #2 2024-04-29 21:01:59 ~10 min ios 📱ipa 📲
6625f89 #3 2024-04-30 10:55:10 ~3 min tests 📄log
✔️ 6625f89 #3 2024-04-30 10:57:23 ~5 min android 🤖apk 📲
✔️ 6625f89 #3 2024-04-30 10:59:52 ~8 min android-e2e 🤖apk 📲
✔️ 6625f89 #3 2024-04-30 11:01:08 ~9 min ios 📱ipa 📲
✔️ f93c183 #4 2024-04-30 15:24:53 ~4 min tests 📄log
✔️ f93c183 #4 2024-04-30 15:27:42 ~7 min android 🤖apk 📲
✔️ f93c183 #4 2024-04-30 15:28:31 ~8 min android-e2e 🤖apk 📲
✔️ f93c183 #4 2024-04-30 15:29:58 ~9 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ f3acfab #5 2024-05-06 07:50:31 ~4 min tests 📄log
✔️ f3acfab #5 2024-05-06 07:53:06 ~7 min android-e2e 🤖apk 📲
✔️ f3acfab #5 2024-05-06 07:54:33 ~8 min android 🤖apk 📲
✔️ f3acfab #5 2024-05-06 07:56:00 ~9 min ios 📱ipa 📲
✔️ 9b3dfad #6 2024-05-06 13:00:28 ~4 min tests 📄log
✔️ 9b3dfad #6 2024-05-06 13:03:53 ~8 min android-e2e 🤖apk 📲
✔️ 9b3dfad #6 2024-05-06 13:04:05 ~8 min android 🤖apk 📲
✔️ 9b3dfad #6 2024-05-06 13:05:38 ~9 min ios 📱ipa 📲

@@ -96,4 +96,4 @@
:customization-color customization-color
:derivation-path path
:keypair-name keypair-name
:on-press #(js/alert "To be implemented")}])]))
}])]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Trailing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!

Comment on lines 10 to 15
(def overview-container
{:height 86})
{:height 82})

(def accounts-list
{:padding-top 32
:padding-bottom 12
:max-height 112})
{:padding-top 8
:max-height 92})
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious about these height/padding changes. Is it from the designs?

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 checked everything with figma 1-1 and it was sitting right like this from what I could see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, I think I will double check this again. If anything we should not be setting max-height like this and using margins for the spacing instead. The height of the cards changes depending on the content used. Same for many of the components. Unfortunately for a lot of these quo components we were using height as margin but this is not desirable. Will revisit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed this @OmarBasem - I instead use margin/padding to handle the spacing there so it is dynamic now 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for checking and fixing this @J-Son89

Btw, I think the default value of flex-grow is 0

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 think in most cases yes but here I think the wrapper component is doing something as I needed to add this.

::community.edit-account-selection (enabled-in-env? :FLAG_EDIT_ACCOUNT_SELECTION_ENABLED)
::wallet.contacts (enabled-in-env? :FLAG_CONTACTS_ENABLED)}))
::wallet.remove-account (enabled-in-env? :FLAG_REMOVE_ACCOUNT_ENABLED)
}))
Copy link
Contributor

Choose a reason for hiding this comment

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

Trailing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed this, thanks!

@J-Son89 J-Son89 force-pushed the jc/hide-graph branch 2 times, most recently from ad88004 to 6625f89 Compare April 30, 2024 10:51
@J-Son89
Copy link
Contributor Author

J-Son89 commented Apr 30, 2024

adding skip qa as it's just a UI change to remove the graph.
I don't think this needs a design review as we will likely polish this page again very soon so no need to waste resources on it but fine to do if requested 👍

cc @status-im/mobile-qa

@J-Son89 J-Son89 merged commit 5802097 into develop May 6, 2024
6 checks passed
@J-Son89 J-Son89 deleted the jc/hide-graph branch May 6, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants