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

Home page transaction entry #198

Merged
merged 24 commits into from
Nov 28, 2023
Merged

Home page transaction entry #198

merged 24 commits into from
Nov 28, 2023

Conversation

Sirmorrison
Copy link
Collaborator

@Sirmorrison Sirmorrison commented Oct 21, 2023

fix #180

image image image image image

@dreacot
Copy link
Member

dreacot commented Oct 23, 2023

Screenshot from 2023-10-23 11-20-53

this should be a wallet dropdown selector

i should be able to select the wallet i want to view transactions for

Screenshot from 2023-10-23 11-21-27

@dreacot
Copy link
Member

dreacot commented Oct 23, 2023

clicking on a tx causes a crash

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x12e424c]

goroutine 181 [running]:
github.com/crypto-power/cryptopower/ui/page/transaction.NewTransactionDetailsPage(0xc0043f7c20, 0xc005180000, 0x18?)
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/ui/page/transaction/transaction_details_page.go:121 +0xd4c
github.com/crypto-power/cryptopower/ui/page/transaction.(*TransactionsPage).HandleUserInteractions(0xc0005b2370)
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/ui/page/transaction/transactions_page.go:413 +0xd6
github.com/crypto-power/cryptopower/ui/page/root.(*HomePage).HandleUserInteractions(0xc000004f00)
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/ui/page/root/home_page.go:186 +0x67
github.com/crypto-power/cryptopower/ui.(*Window).handleFrameEvent(0xc000189860, {{0xc145c8d933aa1f39, 0x2ad18a474f, 0x2ebb220}, {0x3f800000, 0x3f800000}, {0x320, 0x28a}, {0x0, 0x0, ...}, ...})
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/ui/window.go:233 +0x14c
github.com/crypto-power/cryptopower/ui.(*Window).HandleEvents(0xc000189860)
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/ui/window.go:206 +0x2b9
main.main.func3()
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/main.go:110 +0x1d
created by main.main
        /home/kennedy/Projects/src/github.com/cryptopower/cryptopower/main.go:108 +0x61b
exit status 2

@JustinBeBoy
Copy link
Collaborator

app stuck when change filter to sent.

image

Transferred notice "(0)" but we have data on list.

image

the same above

image

"All" filter notice 2 but empty in list

image

@JustinBeBoy
Copy link
Collaborator

The right margin needs alignment

image

@JustinBeBoy
Copy link
Collaborator

App stuck when switch between wallets

image

Please warn users when choosing a wallet that is not synchronized like this:
image

@dreacot
Copy link
Member

dreacot commented Oct 24, 2023

we don't need to show the wallet balance, this is simply a wallet dropdown

we had something like this in godcr
Screenshot from 2023-10-24 11-36-04

selecting the wallet should be a dropdown not open a modal, see the screenshot above
Screenshot from 2023-10-24 11-38-26

@dreacot
Copy link
Member

dreacot commented Oct 24, 2023

staking activity doesn't show when selected on the tx page inside a selected wallet

Screenshot from 2023-10-24 11-40-59

@Sirmorrison
Copy link
Collaborator Author

app stuck when change filter to sent.

image

Transferred notice "(0)" but we have data on list.

image

the same above

image

"All" filter notice 2 but empty in list

image

I could not reproduce the part where the app gets stuck besides that, I attended to all other comments.

@JustinBeBoy
Copy link
Collaborator

Layout break when choose not synced wallet

image

@JustinBeBoy
Copy link
Collaborator

The right margin needs alignment

image

it not resolve

Copy link
Collaborator

@JustinBeBoy JustinBeBoy left a comment

Choose a reason for hiding this comment

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

please check my comments

@Sirmorrison
Copy link
Collaborator Author

Sirmorrison commented Oct 26, 2023

Layout break when choose not synced wallet

image

By layout break, how do you mean? If you mean you just see the red text, thats the design because we cannot using the overlay would involve changing how the overlay is displayed from a selected wallet view.

@JustinBeBoy
Copy link
Collaborator

Layout break when choose not synced wallet
image

By layout break, how do you mean? If you mean you just see the red text, thats the design because we cannot using the overlay would involve changing how the overlay is displayed from a selected wallet view.

Sorry, I mean tab's title not center when choose not sync wallet
image

@Sirmorrison
Copy link
Collaborator Author

The right margin needs alignment
image

it not resolve

This is as a result of the scroll list.. there is an extra padding for the scroll bar. I will see if i can adjust it as its currently on master not my PR.

@JustinBeBoy
Copy link
Collaborator

The right margin needs alignment
image

it not resolve

This is as a result of the scroll list.. there is an extra padding for the scroll bar. I will see if i can adjust it as its currently on master not my PR.

I think we should do it on this PR, so it will be perfect

@dreacot
Copy link
Member

dreacot commented Oct 26, 2023

why not make use of the existing overlay for this situation

Screenshot from 2023-10-26 12-15-40

Screenshot from 2023-10-26 12-17-06

@dreacot
Copy link
Member

dreacot commented Oct 26, 2023

i was on the revocation dropdown on the staking segmented control, switching the segmented control to transaction overview, showed the error seen in the screenshot below

Screenshot from 2023-10-26 12-17-47

and then after clicking okay, the loader just keep showing as seen below

Screenshot from 2023-10-26 12-19-00

@dreacot
Copy link
Member

dreacot commented Oct 26, 2023

trying to switch the wallet selector freezes the app

Screenshot from 2023-10-26 12-25-51

@Sirmorrison
Copy link
Collaborator Author

trying to switch the wallet selector freezes the app

Screenshot from 2023-10-26 12-25-51

I am unable to reproduce this.

@Sirmorrison
Copy link
Collaborator Author

i was on the revocation dropdown on the staking segmented control, switching the segmented control to transaction overview, showed the error seen in the screenshot below

Screenshot from 2023-10-26 12-17-47

and then after clicking okay, the loader just keep showing as seen below

Screenshot from 2023-10-26 12-19-00

resolved

why not make use of the existing overlay for this situation

Screenshot from 2023-10-26 12-15-40

Screenshot from 2023-10-26 12-17-06

Resolved

The right margin needs alignment
image

it not resolve

This is as a result of the scroll list.. there is an extra padding for the scroll bar. I will see if i can adjust it as its currently on master not my PR.

I think we should do it on this PR, so it will be perfect

resolved

@dreacot
Copy link
Member

dreacot commented Oct 29, 2023

the transaction item isn't showing the correct wallet name

Screenshot 2023-10-29 at 2 28 43 PM

@dreacot
Copy link
Member

dreacot commented Oct 29, 2023

trying to switch the wallet selector freezes the app
Screenshot from 2023-10-26 12-25-51

I am unable to reproduce this.

i'd get a screen recording, this happens when i select wallets of different assets from the dropdown about 4 times in a row

@dreacot
Copy link
Member

dreacot commented Oct 29, 2023

trying to switch the wallet selector freezes the app
Screenshot from 2023-10-26 12-25-51

I am unable to reproduce this.

i'd get a screen recording, this happens when i select wallets of different assets from the dropdown about 4 times in a row

Screen.Recording.2023-10-29.at.2.44.43.PM.mov

@dreacot
Copy link
Member

dreacot commented Oct 29, 2023

trying to switch the wallet selector freezes the app
Screenshot from 2023-10-26 12-25-51

I am unable to reproduce this.

i'd get a screen recording, this happens when i select wallets of different assets from the dropdown about 4 times in a row

Screen.Recording.2023-10-29.at.2.44.43.PM.mov

seems like this occurs when btc goes into recovery mode, might not be directly related to this PR

@JustinBeBoy
Copy link
Collaborator

I still stuck when change tab to Staking Activities
I think it occurs when you have many transactions. I have 5 time. It always occurs

image

@JustinBeBoy
Copy link
Collaborator

I have 2 Revocation when select all filter. but when select Revocation it show only one

image

image

@Sirmorrison
Copy link
Collaborator Author

follow up PR #268

clickable := d.clickable
if d.isOpen {
radius = Radius(0)
if item.clickable != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you basically saying that item cannot be nil if d.isOpen?
Because in line 159-162 above, we only give item a value if itemIndex > -1. So if itemIndex is -1, item will be nil.

And this point where you do if item.clickable != nil will panic. Unless d.isOpen is only true if itemIndex > -1.

clickable := d.clickable
if d.isOpen {
radius = Radius(0)
if item.clickable != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

All Possible areas where items could panic was addressed.

I don't see where this is done. Point me to it.

ui/cryptomaterial/dropdown.go Outdated Show resolved Hide resolved
ui/page/components/components.go Outdated Show resolved Hide resolved
ui/page/components/components.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@itswisdomagain itswisdomagain left a comment

Choose a reason for hiding this comment

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

I'm yet to fully test but the code changes look okay now. Just these 2 comments.

ui/page/components/items_scroll.go Outdated Show resolved Hide resolved
ui/page/transaction/transactions_page.go Outdated Show resolved Hide resolved
@dreacot dreacot merged commit c2c43af into crypto-power:master Nov 28, 2023
1 check passed
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.

Create a transactions tab entry entry after the overview tab
4 participants