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

Jump to bottom

feat(@desktop/wallet): Collectible Details balance tag #14204

Merged
merged 1 commit into from
Apr 3, 2024
Merged

feat(@desktop/wallet): Collectible Details balance tag #14204

merged 1 commit into from
Apr 3, 2024

Conversation

Khushboo-dev-cpp
Copy link
Contributor

@Khushboo-dev-cpp Khushboo-dev-cpp commented Mar 27, 2024 •
edited
Loading

fixes #13809

What does the PR do

Implements the balances tag in collectible details for erc1155 and the stacked image behaviour based on balance count

Affected areas

CollectibleDetailsView

StatusQ checklist

  • add documentation if necessary (new component, new feature)
  • update sandbox app
    • in case of new component, add new component page
    • in case of new features, add variation to existing component page
    • nice to have: add it to the demo application as well
  • 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
Screen.Recording.2024-03-27.at.9.37.07.PM.mov
Screen.Recording.2024-04-03.at.5.39.48.PM.mov

note there is an issue with the balances tag not being visible under a white background to be addresses under #14203

@Khushboo-dev-cpp Khushboo-dev-cpp marked this pull request as ready for review March 27, 2024 21:56
@status-im-auto
Copy link
Member

status-im-auto commented Mar 27, 2024 •
edited
Loading

Jenkins Builds

Click to see older builds (19)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b7e96d3 #1 2024-03-27 22:02:55 ~6 min tests/nim 📄log
✔️ b7e96d3 #1 2024-03-27 22:03:29 ~7 min macos/aarch64 🍎dmg
✔️ b7e96d3 #1 2024-03-27 22:06:27 ~10 min macos/x86_64 🍎dmg
✔️ b7e96d3 #1 2024-03-27 22:07:26 ~11 min tests/ui 📄log
✔️ b7e96d3 #1 2024-03-27 22:13:42 ~17 min linux/x86_64 📦tgz
✔️ b7e96d3 #1 2024-03-27 22:20:37 ~24 min windows/x86_64 💿exe
✔️ d5f425c #2 2024-04-02 08:57:06 ~6 min tests/nim 📄log
✔️ d5f425c #2 2024-04-02 08:57:13 ~6 min macos/aarch64 🍎dmg
✔️ d5f425c #2 2024-04-02 08:58:44 ~8 min macos/x86_64 🍎dmg
✔️ d5f425c #2 2024-04-02 09:01:36 ~11 min tests/ui 📄log
✔️ d5f425c #2 2024-04-02 09:06:19 ~15 min linux/x86_64 📦tgz
✔️ d5f425c #2 2024-04-02 09:14:45 ~24 min windows/x86_64 💿exe
✔️ 4f473d7 #3 2024-04-02 10:01:35 ~7 min tests/nim 📄log
✔️ 4f473d7 #3 2024-04-02 10:02:11 ~7 min macos/x86_64 🍎dmg
✔️ 4f473d7 #3 2024-04-02 10:05:30 ~11 min macos/aarch64 🍎dmg
❌ 4f473d7 #3 2024-04-02 10:05:51 ~11 min tests/ui 📄log
✔️ 4f473d7 #3 2024-04-02 10:11:11 ~16 min linux/x86_64 📦tgz
✔️ 4f473d7 #3 2024-04-02 10:19:41 ~25 min windows/x86_64 💿exe
❌ 4f473d7 #4 2024-04-03 08:18:32 ~12 min tests/ui 📄log
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 1961b98 #4 2024-04-03 10:11:46 ~5 min macos/aarch64 🍎dmg
✔️ 1961b98 #4 2024-04-03 10:12:37 ~6 min tests/nim 📄log
✔️ 1961b98 #4 2024-04-03 10:13:56 ~7 min macos/x86_64 🍎dmg
❌ 1961b98 #5 2024-04-03 10:17:09 ~11 min tests/ui 📄log
✔️ 1961b98 #4 2024-04-03 10:21:18 ~15 min linux/x86_64 📦tgz
✔️ 1961b98 #4 2024-04-03 10:29:29 ~23 min windows/x86_64 💿exe
❌ 1961b98 #6 2024-04-03 10:52:26 ~10 min tests/ui 📄log
❌ 1961b98 #7 2024-04-03 11:12:42 ~10 min tests/ui 📄log
❌ 1961b98 #8 2024-04-03 11:24:13 ~10 min tests/ui 📄log
✔️ 5424eda #5 2024-04-03 14:07:52 ~5 min macos/aarch64 🍎dmg
✔️ 5424eda #5 2024-04-03 14:09:49 ~6 min tests/nim 📄log
✔️ 5424eda #9 2024-04-03 14:13:55 ~11 min tests/ui 📄log
✔️ 5424eda #5 2024-04-03 14:18:23 ~15 min macos/x86_64 🍎dmg
✔️ 5424eda #5 2024-04-03 14:18:42 ~15 min linux/x86_64 📦tgz
✔️ 5424eda #5 2024-04-03 14:25:53 ~22 min windows/x86_64 💿exe

stefandunca
stefandunca reviewed Apr 1, 2024
View reviewed changes
Copy link
Contributor

@stefandunca stefandunca left a comment

Choose a reason for hiding this comment

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

I like the visual hint for multiple collectibles.

I'm wondering what is design's take on the fixed aspect ration for the frame? It looks to me a little off for the case when the image's aspect ratio doesn't match the frame. I could not quickly find the design on this matter so I'm thinking that maybe the frame should follow the image contour.
Here is a side by side from the demo with design found in the issue hoping that will help to clarify my view point.

Implementation Design
image image

@Khushboo-dev-cpp
Copy link
Contributor Author

I like the visual hint for multiple collectibles.

I'm wondering what is design's take on the fixed aspect ration for the frame? It looks to me a little off for the case when the image's aspect ratio doesn't match the frame. I could not quickly find the design on this matter so I'm thinking that maybe the frame should follow the image contour. Here is a side by side from the demo with design found in the issue hoping that will help to clarify my view point.

Implementation Design
image image

@benjthayer can you please how it looks when the collectible image doesnt fill the entire box? is this okay from design perspective?

@Khushboo-dev-cpp Khushboo-dev-cpp force-pushed the feat/#13809 branch 2 times, most recently from d5f425c to 4f473d7 Compare April 2, 2024 09:54
@benjthayer
Copy link

I like the visual hint for multiple collectibles.
I'm wondering what is design's take on the fixed aspect ration for the frame? It looks to me a little off for the case when the image's aspect ratio doesn't match the frame. I could not quickly find the design on this matter so I'm thinking that maybe the frame should follow the image contour. Here is a side by side from the demo with design found in the issue hoping that will help to clarify my view point.
Implementation Design
image image

@benjthayer can you please how it looks when the collectible image doesnt fill the entire box? is this okay from design perspective?

Yeah this was on my to do list to confirm how we should handle single and multiple horizontal and portrait aspect ratios. Will revert with designs 👍

@Khushboo-dev-cpp
Copy link
Contributor Author

I like the visual hint for multiple collectibles.
I'm wondering what is design's take on the fixed aspect ration for the frame? It looks to me a little off for the case when the image's aspect ratio doesn't match the frame. I could not quickly find the design on this matter so I'm thinking that maybe the frame should follow the image contour. Here is a side by side from the demo with design found in the issue hoping that will help to clarify my view point.
Implementation Design
image image

@benjthayer can you please how it looks when the collectible image doesnt fill the entire box? is this okay from design perspective?

Yeah this was on my to do list to confirm how we should handle single and multiple horizontal and portrait aspect ratios. Will revert with designs 👍

I think we can handle these design changes here in this task #14203

@Khushboo-dev-cpp Khushboo-dev-cpp mentioned this pull request Mar 27, 2024
Closed
stefandunca
stefandunca approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@stefandunca stefandunca left a comment

Choose a reason for hiding this comment

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

👍

@Khushboo-dev-cpp Khushboo-dev-cpp merged commit 2d20a2e into master Apr 3, 2024
8 checks passed
@Khushboo-dev-cpp Khushboo-dev-cpp deleted the feat/#13809 branch April 3, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caybro caybro caybro left review comments

@stefandunca stefandunca stefandunca approved these changes

@dlipicar dlipicar dlipicar approved these changes

@alaibe alaibe Awaiting requested review from alaibe

@Cuteivist Cuteivist Awaiting requested review from Cuteivist

@IvanBelyakoff IvanBelyakoff Awaiting requested review from IvanBelyakoff

@saledjenic saledjenic Awaiting requested review from saledjenic

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Collectible Details balance tag
6 participants
@Khushboo-dev-cpp @status-im-auto @benjthayer @caybro @dlipicar @stefandunca