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

fix(TokenUI): Hide explicitly ManageTokensCommunityTag in community settings page #14500

Conversation

Seitseman
Copy link
Contributor

@Seitseman Seitseman commented Apr 23, 2024

What does the PR do

PR hide the ManageTokensCommunityTag item in "Community"->Tokens page. Previous changes introduced ManageTokensCommunityTag 12519 ManageTokensCommunityTag for displaying community icon and name when showing Collectibles in User's Wallet->Collectibles page. However when going to particular Community and checking its "Tokens" there is no need to display that tag, according to the MintedTokensSettingsPanel PR: 11347

Affected areas

Wallet
Communities

StatusQ checklist

  • [x ] test changes in both light and dark theme?

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it

image

In Wallet -> Collectibles, ManageTokensCommunityTag is displayed:

image

@status-im-auto
Copy link
Member

status-im-auto commented Apr 23, 2024

Jenkins Builds

Click to see older builds (18)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 11f95ac #1 2024-04-23 13:00:56 ~7 min macos/aarch64 🍎dmg
✔️ 11f95ac #1 2024-04-23 13:02:35 ~9 min tests/nim 📄log
✔️ 11f95ac #1 2024-04-23 13:05:23 ~12 min tests/ui 📄log
✔️ 11f95ac #1 2024-04-23 13:08:39 ~15 min macos/x86_64 🍎dmg
✔️ 11f95ac #1 2024-04-23 13:15:14 ~22 min linux/x86_64 📦tgz
✔️ 11f95ac #1 2024-04-23 13:26:14 ~33 min windows/x86_64 💿exe
✔️ a3b327a #2 2024-04-24 10:00:01 ~4 min macos/aarch64 🍎dmg
✔️ a3b327a #2 2024-04-24 10:03:23 ~7 min macos/x86_64 🍎dmg
✔️ a3b327a #2 2024-04-24 10:05:31 ~9 min tests/nim 📄log
✔️ a3b327a #2 2024-04-24 10:12:34 ~17 min tests/ui 📄log
✔️ a3b327a #2 2024-04-24 10:15:09 ~19 min linux/x86_64 📦tgz
✔️ a3b327a #2 2024-04-24 10:27:28 ~31 min windows/x86_64 💿exe
✔️ 1570e50 #4 2024-04-26 13:36:59 ~7 min macos/aarch64 🍎dmg
✔️ 1570e50 #4 2024-04-26 13:39:02 ~9 min tests/nim 📄log
✔️ 1570e50 #4 2024-04-26 13:40:15 ~10 min macos/x86_64 🍎dmg
✔️ 1570e50 #4 2024-04-26 13:46:11 ~16 min tests/ui 📄log
✔️ 1570e50 #4 2024-04-26 13:46:19 ~16 min linux/x86_64 📦tgz
✔️ 1570e50 #4 2024-04-26 14:03:50 ~33 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a3cb7c0 #5 2024-05-08 09:15:44 ~7 min macos/aarch64 🍎dmg
✔️ a3cb7c0 #5 2024-05-08 09:16:40 ~8 min macos/x86_64 🍎dmg
✔️ a3cb7c0 #5 2024-05-08 09:17:44 ~9 min tests/nim 📄log
✔️ a3cb7c0 #5 2024-05-08 09:24:43 ~16 min tests/ui 📄log
✔️ a3cb7c0 #5 2024-05-08 09:27:51 ~19 min linux/x86_64 📦tgz
✔️ a3cb7c0 #5 2024-05-08 09:42:15 ~34 min windows/x86_64 💿exe
✔️ 6efc043 #6 2024-05-08 10:50:43 ~4 min macos/aarch64 🍎dmg
✔️ 6efc043 #6 2024-05-08 10:53:22 ~7 min macos/x86_64 🍎dmg
✔️ 6efc043 #6 2024-05-08 10:56:10 ~10 min tests/nim 📄log
✔️ 6efc043 #6 2024-05-08 11:03:34 ~17 min tests/ui 📄log
✔️ 6efc043 #6 2024-05-08 11:06:10 ~20 min linux/x86_64 📦tgz
✔️ 6efc043 #6 2024-05-08 11:18:32 ~32 min windows/x86_64 💿exe

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@noeliaSD noeliaSD left a comment

Choose a reason for hiding this comment

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

I've tested the flow and it works properly but I have some comments about the solution!

@Seitseman
Copy link
Contributor Author

Checking Figma Communities P3 -> Tokens , it looks like indeed communityId should not be set on MintedTokensView component.
I will do the changes according to @noeliaSD suggestion

Copy link
Contributor

@noeliaSD noeliaSD left a comment

Choose a reason for hiding this comment

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

Tested again and works fine!

Thanks for addressing the comments! I've added some other minor ones, please take a look. Otherwise, LGTM!

You will need to rebase on top of master to make the e2e tests work properly since there have been some changes there that are making this pr failing in this job!

@@ -178,6 +178,7 @@ Control {
}

ManageTokensCommunityTag {
id: manageTokenCommunityTag
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 need then this id here?

ui/app/AppLayouts/Communities/views/MintedTokensView.qml Outdated Show resolved Hide resolved
@Seitseman Seitseman force-pushed the 13220-token-ui-is-corrupted-in-the-community-tokens-settings branch from 1570e50 to a3cb7c0 Compare May 8, 2024 09:07
…ettings page according to collectibles cells sizing
@Seitseman Seitseman force-pushed the 13220-token-ui-is-corrupted-in-the-community-tokens-settings branch from a3cb7c0 to 6efc043 Compare May 8, 2024 10:45
@Seitseman Seitseman merged commit 05000c5 into master May 8, 2024
8 checks passed
@Seitseman Seitseman deleted the 13220-token-ui-is-corrupted-in-the-community-tokens-settings branch May 8, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Token UI is corrupted in the Community->Tokens settings
5 participants