MXCzkEVM / datadash-wallet

Other
0 stars 1 forks source link

Customize dApp overview #188

Open reasje opened 2 months ago

reasje commented 2 months ago

Image

Image

https://www.figma.com/file/s1EaHblSIZm29z59KQcM5g/AXS-App-Homescreen-Upgrade?type=design&node-id=0-1&mode=design&t=MVy9YROXyHztHg9J-0

reasje commented 2 months ago

@sheenhx Looks like I don't have full permission. Can you plz grant It?

Image

reasje commented 2 months ago

I have implemented new UI, Only Cupertino context menu is left off, I will finish today and I have to start the most challenging of this ticket which is about reordering the paged list view which the reordering is done between multiple pages.

reasje commented 2 months ago

I am getting this bug, I checking the reason,


I/flutter ( 9386): The following assertion was thrown building Provider<ColorsTheme>(value: Instance of
I/flutter ( 9386): 'ColorsThemeDark', message: 'package:flutter/src/widgets/framework.dart': Failed assertion: line
framework.dart:1
I/flutter ( 9386): 5541 pos 14: '() {\n        // check that it really is our descendant\n        Element? ancestor =
I/flutter ( 9386): dependent._parent;\n        while (ancestor != this && ancestor != null) {\n          ancestor =
I/flutter ( 9386): ancestor._parent;\n        }\n        return ancestor == this;\n      }()': is not true.\nSee also:
I/flutter ( 9386): https://flutter.dev/docs/testing/errors, dirty, renderObject: RenderErrorBox#8001a NEEDS-LAYOUT
I/flutter ( 9386): NEEDS-PAINT DETACHED):
I/flutter ( 9386): 'package:flutter/src/widgets/binding.dart': Failed assertion: line 1207 pos 12: 'renderObject.child
binding.dart:1207
I/flutter ( 9386): == child': is not true.
I/flutter ( 9386):
I/flutter ( 9386): Either the assertion indicates an error in the framework itself, or we should provide substantially
I/flutter ( 9386): more information in this error message to help you determine and fix the underlying cause.
I/flutter ( 9386): In either case, please report this assertion by filing a bug on GitHub:
I/flutter ( 9386):   https://github.com/flutter/flutter/issues/new?template=2_bug.md````
reasje commented 2 months ago

Still not able to fix, According to my search the error doesn't indicate a specific problem.

reasje commented 2 months ago

I was able to have GridView with horizontal scrolling and also reordering feature, I am working paged behavior and multi page reodering behavior.

reasje commented 2 months ago

paged behavior and multi page reodering is done but there are some animation issues not sure what It's related to but I will try to integrate current test project with AXS wallet and try to check the animations and behaviors there.

reasje commented 2 months ago

I have integrated the new reodering implementation, I have started debugging and I am working on UI once I finish this I will start impl logic for order managing & caching.

reasje commented 2 months ago

I am getting this bug, I checking the reason,

I/flutter ( 9386): The following assertion was thrown building Provider<ColorsTheme>(value: Instance of
I/flutter ( 9386): 'ColorsThemeDark', message: 'package:flutter/src/widgets/framework.dart': Failed assertion: line
framework.dart:1
I/flutter ( 9386): 5541 pos 14: '() {\n        // check that it really is our descendant\n        Element? ancestor =
I/flutter ( 9386): dependent._parent;\n        while (ancestor != this && ancestor != null) {\n          ancestor =
I/flutter ( 9386): ancestor._parent;\n        }\n        return ancestor == this;\n      }()': is not true.\nSee also:
I/flutter ( 9386): https://flutter.dev/docs/testing/errors, dirty, renderObject: RenderErrorBox#8001a NEEDS-LAYOUT
I/flutter ( 9386): NEEDS-PAINT DETACHED):
I/flutter ( 9386): 'package:flutter/src/widgets/binding.dart': Failed assertion: line 1207 pos 12: 'renderObject.child
binding.dart:1207
I/flutter ( 9386): == child': is not true.
I/flutter ( 9386):
I/flutter ( 9386): Either the assertion indicates an error in the framework itself, or we should provide substantially
I/flutter ( 9386): more information in this error message to help you determine and fix the underlying cause.
I/flutter ( 9386): In either case, please report this assertion by filing a bug on GitHub:
I/flutter ( 9386):   https://github.com/flutter/flutter/issues/new?template=2_bug.md````

The bug was related to UI but deep inside Flutter widget keys the stateless widget was repeating It' key and It was causing bug in gridview.

reasje commented 2 months ago

I have finished some of UI bugs and implemented logic and order caching, Currently I am debugging this feature. Things that I think need to be done :

Reorder logic & cache test Reorder animation card background dynamic counts for Tablet & Mobile Cupertino Context menue

reasje commented 2 months ago

I have finished one point and made the card background transparetn, dynamic counts for Tablet & Mobile is in test stage.

reasje commented 2 months ago

Dynamic counts for Tablet & Mobile are done, I have finished cupertino context menu and I have started reoder logic & cache test

reasje commented 2 months ago

Reorder Logic & caching is almost done some small bugs are being fixed, Getting Favicon of bookmarks are added, For doing so websites had different places for favicons so i used a package to do that. I will add the new icons to dapps and also pagination indicator need some changes to be working with new design, Once those are I can release a stable version including this feature

reasje commented 2 months ago

@SanghamitraBhowmick1993 I have just pushed a release for dapps overview Figma link is in above comments for checking UI Plz ignore color miss matches for now I will need to talk to Ruben about this.

reasje commented 1 month ago

@SanghamitraBhowmick1993 I have just pushed a update that included support for new icons, Plz note that some dapps yet have no icon so the Tether is the default one for now.

SanghamitraBhowmick1993 commented 1 month ago

Few observations:

1

There's a font mismatch. I think the "Remove app" button should be red in color. The lines between the sections are not very distinct. Also the dimensions of the popup look slightly different?

Android

Image

iOS

Image

2

The remove popup has some mismatches

Image

3

The popup positioning isn't correct. It consistently opens all dapps in the middle, whereas it should open the popup next to the respective dapp.

Image

4

I can't find the (-) icon for all dapps, although I can drag and reposition them. Is this intentional? Figma shows the (-) icon for all dapps.

Image

reasje commented 1 month ago
  1. I will check and report
  2. This was a change me and Ruben agreed on to use the AXS dialog instead of native IOS dialog
  3. This is also something we agreed on
  4. It's not expected to get ( - ) icon for default dapps, Because we cannnot remove them, It's only for bookmarks
reasje commented 1 month ago

@SanghamitraBhowmick1993 I also detected a problem with page indicator, I was on Geneva open home screen moved to wallet page open home screen again

Image

reasje commented 1 month ago

@SanghamitraBhowmick1993 Above bug also has been fixed Also sizes of dapps on Ipad has been improved

a. The context menu font is the AXS default font using IOS font will have negetaive impact on app UI/UX b. The colors should be fixed in the next release c. I have done tests in IOS, The lines in IOS is diffrerent than Android, I will check to see if there is a solution to make them look alike, So far no luck I have found issues that are open and prove that there is no fix for that. d. I have added new scaling animation and increased the size of icon when It's zoomed just improve the UI, Let me know what you think.

reasje commented 1 month ago

@SanghamitraBhowmick1993 Thanks to Ruben I have added new icons, All apps should have their icon.

SanghamitraBhowmick1993 commented 1 month ago

I think the previous deployment was unsuccessful @reasje

Image

Also a few more observations

1

The 'Permission Use Case Popup' overlaps with the 'Hold Down App Icon'.

Image

2

Should the dapp label names and icons be validated against Figma, or are they aligned with someone according to your implementation? I noticed some mismatches.

Image

cc: @sheenhx for observation #2

reasje commented 1 month ago

@SanghamitraBhowmick1993 New build successful on Android https://appcenter.ms/orgs/MXC-Foundation-gGmbH/apps/DD-Wallet-Android/build/branches/pre_main_qa/builds/465 Checking the error on IOS.

reasje commented 1 month ago

I think the previous deployment was unsuccessful @reasje

Image

Also a few more observations

#1 The 'Permission Use Case Popup' overlaps with the 'Hold Down App Icon'.

Image

#2 Should the dapp label names and icons be validated against Figma, or are they aligned with someone according to your implementation? I noticed some mismatches.

Image

cc: @sheenhx for observation #2

@SanghamitraBhowmick1993

  1. I will improve the behavior, Both behaviors should happen at one time
  2. I am using json names from dapp store, These are not hard coded, If any change is required we need to make the changes on the dapp store repo
reasje commented 1 month ago

@SanghamitraBhowmick1993 IOS build succeeded https://appcenter.ms/orgs/MXC-Foundation-gGmbH/apps/DD-Wallet-iOS/build/branches/pre_main_qa/builds/417

reasje commented 1 month ago

@SanghamitraBhowmick1993 Just pushed a fix for dapp being opened while the context menu is open And also added a shatter animation for bookmarks when they are being removed.

SanghamitraBhowmick1993 commented 1 month ago

For : The lines between the sections are not very distinct. Comment for @reasje : To get the expected implementation might need more technical effort and solutioning.

Current view:

Image

cc: @sheenhx

reasje commented 1 month ago

@SanghamitraBhowmick1993 I have noticed a senario which is potential for producing bugs,

  1. Install versions before new dpps overview feat
  2. Try adding a bookmark
  3. Install the new version with dapps overview (Version that doesn't include the fix)
  4. The dapps image should be black To fix the problem install the version with fix.
reasje commented 1 month ago

@SanghamitraBhowmick1993 Just merged the fix for line thickness.