Fyne History Page Implementation #439

Sirmorrison · 2019-11-05T18:33:55Z

Fixes #439

oshorefueled · 2019-12-15T01:13:43Z

The history page seems to work when using one wallet. However, it's not fully functional with multiple wallets. First, how multiple wallets are displayed. A dropdown would be more suitable for displaying users' wallets. You can use the "receive" page as a reference for that.

I get an empty transactions result when I try switching between wallets and that's just one of the issues. Kindly create multiple wallets and test.

terminal/pages/history.go

cli/go.sum

fyne/helpers/amount.go

oshorefueled · 2019-12-21T06:53:28Z

Please remove this file https://github.com/raedahgroup/godcr/blob/28a531acf89b55f9ab87a2e920240d443c6cef9b/cmd/godcr-fyne/.~bash-profile.swp from your PR

… basedd on review, minor code clean up

…nt function, cleaned up code

…tails popup, cleaned up code

…ed copy info etc

… when a filter with smaler no of tx

…wn when they are initialized

metaclips

You should try to split components to separate functions and place them in the handler package for easy review also scrolling seems very slow, can we reduce transaction table per scroll to 15 instead of 25?

fyne/pages/history.go

Sirmorrison · 2019-12-21T22:04:33Z

You should try to split components to separate functions and place them in the handler package for easy review also scrolling seems very slow, can we reduce transaction table per scroll to 15 instead of 25?

As a matter of uniformity, across all godcr interfaces, the txperpage is 25.

metaclips · 2019-12-22T10:09:06Z

You should try to split components to separate functions and place them in the handler package for easy review also scrolling seems very slow, can we reduce transaction table per scroll to 15 instead of 25?

As a matter of uniformity, across all godcr interfaces, the txperpage is 25.

Scrolling is slow and table contents distorts while scrolling we should reduce table contents.

…into fyneHistoryPage

oshorefueled · 2019-12-24T08:49:31Z

Is it necessary that we use a horizontal scroll?

cmd/go.mod

metaclips · 2019-12-24T21:23:35Z

Is it necessary that we use a horizontal scroll?

There are contents we can do away with.
I don't see why the texts are needed with the icons in Direction line, type can also be removed and also hash text can be reduced so that the width wont be jumpy when users switch pages.

Sirmorrison · 2019-12-25T02:06:57Z

Is it necessary that we use a horizontal scroll?

There are contents we can do away with.
I don't see why the texts are needed with the icons in Direction line, type can also be removed and also hash text can be reduced so that the width wont be jumpy when users switch pages.

Is the recommendation that i remove the icon?
About the jumpy pages why dont we give the fyne interface a fixed min width?

Sirmorrison · 2019-12-25T02:16:51Z

Is it necessary that we use a horizontal scroll?

resolved

oshorefueled · 2019-12-27T09:32:55Z

The transaction details popup seems to be losing part of its content to the edges of the window.

cmd/go.mod

oshorefueled · 2019-12-27T09:53:51Z

fyne/widgets/table.go

 }

+// Append widget adds to the bottom row of a table.
 func (table *Table) Append(data ...*widget.Box) {


You changed quite a number of code in the table.go. It's a widget I used on the overview page. It seemed to work well, any reason for the change? Would my existing implementation work without an issue if I used this?

I had this these codes already before the overview page was implemented. Anyways have you tested your implementation on the overview page just to be sure there are no issues because overview page seems fine to me.?

Sirmorrison · 2019-12-28T13:25:32Z

The transaction details popup seems to be losing part of its content to the edges of the window.

so basically the issue is the change @metaclips suggested that we reduce the width of the history page. this also affected the txdetails popup as its wider than the current history page.
planetdecred/godcr#439 (comment)

Two things can done here.

increase the width of the default history page to accommodate the details page.
reduce the width of the details popup and add a horizontal scroll to view the hidden details to the right.

for now i will make it scroll horizontally.

…ndid deletion

metaclips

scrolling seems to be much better, still needs little changes to UI.

Status table contents should be centered.

- On transaction details page, confirmations-Data contents are not aligned, can we place them in a form widget? - All tables on transaction details page should be properly aligned

- Info icon should be right aligned, and popup width should be reduced.

metaclips

You should add an init prefix to component on history page and make the component that is to be initialised the first calling function and other helper functions below on each files for easy review and so that others wont have issues implementing features on the page.

metaclips · 2019-12-28T19:59:22Z

fyne/pages/handler/historypagehandler/txtablecomponents.go

+	}
+
+	if prepend {
+		txTable.Prepend(txBox...)


You should try as much as possible not to use Prepend and Append function especially when users change to history page, can we instead call A NewTable instead for faster rendering , if users view history page?

This will function more like pagination reason is for every update a new table will be called and this will have only data from the last tx fetched.

metaclips · 2019-12-28T19:59:29Z

fyne/pages/handler/historypagehandler/filterselector.go

+
+	txCountForFilter, err := historyPage.MultiWallet.WalletWithID(historyPage.selectedWalletID).CountTransactions(allTxFilters["All"])
+	if err != nil {
+		historyPage.errorMessage = fmt.Sprintf("Cannot load history page page. Error getting transaction count for filter All: %s", err.Error())


why are we passing error messages here? isn't there already a widget that handles that? Also, if err!=nil parameter is met, the app crashes, all component functions that require using dcrlibwallet should have a return parameter "error", you should stop all history page initialisation if any function throws an error.

this is not true if there is an error at any point the error is returned the app does not panic... you should do well to test this properly as i have done so from my end...

if you are changing != to == you should comment out err.Error() else it will panic as there is no error to be returned.

This is what is shown when I make err == nil here, you are showing parts of history page components even when there was an error.
You shouldn't be passing errors to variables and then making a return

You are still calling up functions even if there was error on other functions called recently and then checking if any of the functions returned an error here

This shouldn't be as any of the functions that errors and returns early without initialising struct variable "HistoryPageData struct" could cause a panic in other functions if they use the uninitialised variable, you should instead pass return parameter "errors" to all functions that uses dcrlibwallet also InitHistoryPage function and then if any of the dcrlibwallet library functions return an error,

Stop the failed function, return the error

Stop history page initialisation in the handler package, return the error passed to InitHistoryPage

Check if InitHistoryPage function which is to be called in history.go file returns an err if there was an error, only show on history page a label indicating that there was an error initialising history page.

…contents and other cleanups

Sirmorrison marked this pull request as ready for review December 4, 2019 07:01

Sirmorrison force-pushed the fyneHistoryPage branch from 27f62f5 to 0f77b1d Compare December 8, 2019 22:48

oshorefueled suggested changes Dec 15, 2019

View reviewed changes

terminal/pages/history.go Outdated Show resolved Hide resolved

cli/go.sum Outdated Show resolved Hide resolved

fyne/helpers/amount.go Outdated Show resolved Hide resolved

Sirmorrison force-pushed the fyneHistoryPage branch from 7dec5c7 to 4db9223 Compare December 20, 2019 06:54

Sirmorrison added 24 commits December 21, 2019 13:13

added transaction filter popup

f1dcdfd

fixed dropdown selection issues

eae10b8

added table.go, and table header

d347c73

added table.go, and fetched table data into the table

6af6f32

bug fix with tablefilter

a8a6c51

implemented txtable filtering

cf1a818

added pagination

0f3fa0c

added transaction details page, improved table.go, removed pagination…

2c1c538

… basedd on review, minor code clean up

removed txdetails.go

d19d5f8

added errorhandling, split function into table header and table conte…

95100e2

…nt function, cleaned up code

added copy txhash copy functionality for txdetails page, aligned txde…

4bd5f6d

…tails popup, cleaned up code

added modal popup collapse icon, made txdetail pop a modal popup, add…

c70253a

…ed copy info etc

hint tip

a241b45

renamed variables, remoeved commentedline and un used lines ofcode

60ad5d1

fixed ssues with tx table not scrolling and returning to the table to…

c35e1ba

… when a filter with smaler no of tx

fixed panic issue with error handlers for txdetails page and txdropdo…

3463275

…wn when they are initialized

fixed error handler panic for tx, added txsort dropdown to history page

1e8334c

fixe issues with txdetails popup

c328cb1

un-did changes

6278f21

rebased pr

7540cf9

added transaction filter popup

b5c41bd

fixed dropdown selection issues

f3057c6

added table.go, and table header

b9b10ab

added table.go, and fetched table data into the table

b6de20b

metaclips suggested changes Dec 21, 2019

View reviewed changes

fyne/pages/history.go Outdated Show resolved Hide resolved

fyne/pages/history.go Outdated Show resolved Hide resolved

fyne/pages/history.go Outdated Show resolved Hide resolved

Sirmorrison and others added 6 commits December 22, 2019 20:44

added scroll feature to wallet dropdown

ef81d88

implemented @metaclips review

9f4ce43

removed unrelated file

a8cab7a

removed unrelated file

06b6a8b

minor mod

3bc6ba6

Merge branch 'fyneHistoryPage' of https://github.com/Sirmorrison/godcr …

a7a406b

…into fyneHistoryPage

oshorefueled reviewed Dec 24, 2019

View reviewed changes

cmd/go.mod Outdated Show resolved Hide resolved

Sirmorrison added 3 commits December 25, 2019 03:19

reduced table width, removed horizontal scroll, removed type column

c868cab

fixed issues with no transaction in wallet message not showing

0d86a52

removed direction label and added #column

7e63a18

oshorefueled reviewed Dec 27, 2019

View reviewed changes

fixed issues with txdetails popup being larger than the screensize, u…

c72a35a

…ndid deletion

metaclips suggested changes Dec 28, 2019

View reviewed changes

Sirmorrison added 5 commits December 28, 2019 21:58

trimmed long string and optimized code

1303925

aligned info icon to the right and reduced popup size, aligned table …

a50dcd4

…contents and other cleanups

used form widget for txdetails data

8b9d018

cleaned up code and fixed issues with txtable content alignment

08c678b

fixed pagination

2fb8d2e

oluwandabira added enhancement New feature or request fyne godcr-fyne labels Jan 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fyne History Page Implementation #439

Fyne History Page Implementation #439

Sirmorrison commented Nov 5, 2019 •

edited

Loading

oshorefueled commented Dec 15, 2019

oshorefueled commented Dec 21, 2019

metaclips left a comment

Sirmorrison commented Dec 21, 2019

metaclips commented Dec 22, 2019

oshorefueled commented Dec 24, 2019 •

edited

Loading

metaclips commented Dec 24, 2019

Sirmorrison commented Dec 25, 2019 •

edited

Loading

Sirmorrison commented Dec 25, 2019

oshorefueled commented Dec 27, 2019

oshorefueled Dec 27, 2019

Sirmorrison Dec 28, 2019 •

edited

Loading

Sirmorrison commented Dec 28, 2019 •

edited

Loading

metaclips left a comment •

edited

Loading

metaclips left a comment

metaclips Dec 28, 2019

Sirmorrison Dec 29, 2019

metaclips Dec 28, 2019

Sirmorrison Dec 29, 2019 •

edited

Loading

metaclips Dec 30, 2019

Fyne History Page Implementation #439

Are you sure you want to change the base?

Fyne History Page Implementation #439

Conversation

Sirmorrison commented Nov 5, 2019 • edited Loading

oshorefueled commented Dec 15, 2019

oshorefueled commented Dec 21, 2019

metaclips left a comment

Choose a reason for hiding this comment

Sirmorrison commented Dec 21, 2019

metaclips commented Dec 22, 2019

oshorefueled commented Dec 24, 2019 • edited Loading

metaclips commented Dec 24, 2019

Sirmorrison commented Dec 25, 2019 • edited Loading

Sirmorrison commented Dec 25, 2019

oshorefueled commented Dec 27, 2019

oshorefueled Dec 27, 2019

Choose a reason for hiding this comment

Sirmorrison Dec 28, 2019 • edited Loading

Choose a reason for hiding this comment

Sirmorrison commented Dec 28, 2019 • edited Loading

metaclips left a comment • edited Loading

Choose a reason for hiding this comment

metaclips left a comment

Choose a reason for hiding this comment

metaclips Dec 28, 2019

Choose a reason for hiding this comment

Sirmorrison Dec 29, 2019

Choose a reason for hiding this comment

metaclips Dec 28, 2019

Choose a reason for hiding this comment

Sirmorrison Dec 29, 2019 • edited Loading

Choose a reason for hiding this comment

metaclips Dec 30, 2019

Choose a reason for hiding this comment

Sirmorrison commented Nov 5, 2019 •

edited

Loading

oshorefueled commented Dec 24, 2019 •

edited

Loading

Sirmorrison commented Dec 25, 2019 •

edited

Loading

Sirmorrison Dec 28, 2019 •

edited

Loading

Sirmorrison commented Dec 28, 2019 •

edited

Loading

metaclips left a comment •

edited

Loading

Sirmorrison Dec 29, 2019 •

edited

Loading