KayinCheung / KayinCheung.github.io

EscrowMyEther. Decentralized Escrow System built on Ethereum.
http://escrowmyether.com/
GNU General Public License v3.0
54 stars 30 forks source link

Misaligned Tx History after clicking "next page" or "prev page" #2

Closed KayinCheung closed 7 years ago

KayinCheung commented 7 years ago

On initial load, the transaction history displays properly.

initial load

Whenever next page is clicked, tx history becomes misaligned.

after_next_click

The code that generated Tx history.

getBuyerHistory(e, startID){

    var TableRows = []
    var callInfo
    this.state.mee.buyerHistory(e, startID, function(error, result){
        if(!error)
            {
            callInfo = result
            _.eachRight(callInfo[0], (value, index) => {
                TableRows.push(
                <tr>            
                <td><p className="small">#{this.state.startID + index}</p></td>   
                <td>Purchase from<p className="small">{callInfo[0][index]}</p></td>             
                <td>{+(parseFloat(callInfo[2][index])/(10**18)).toFixed(6)} ETH</td>
                <td>{this.state.ETH_CLIENT.toAscii(callInfo[3][index])}</td>
                </tr>   
                )
            })
            this.setState({                 
                tableRows: TableRows
            });
            }   
        else
            console.log(error)
    }.bind(this))
}

The reason for this issue is likely this output showing varying string length {this.state.ETH_CLIENT.toAscii(callInfo[3][index])}

If every single status is the same (eg, "In progress"), then the issue does not happen. Any tips on fixing this is appreciated.

sogko commented 7 years ago

(Referred from the reddit thread)

The reason why the table in the 2nd image is misaligned is because there are some weird garbage characters appearing in in the Status column for the last row:

screen shot 2017-09-14 at 1 54 03 pm

This made the width of the column longer.

Two issues you need to address:

1) Address why the status Complete contains garbage characters at the end. Either the hex value for callInfo[3][index] is malformed or toAscii() is broken (unlikely though).

(If you can share with me the hex value for that row's Status column, I can help to investigate further. Basically just console.log(callInfo[3][index]) probably would be sufficient).

2) Style the table to handle different statuses with differing length with appropriate CSS. A lot of possible approaches; one possible approach: make the Status column fixed to the left with a fixed width and allow text in that column to overflow or wrap.

Note: I tried to replicate your issue (created 26 transactions with 1 complete, others in-progress), but unable to reproduce.

Screenshot below:

screen shot 2017-09-14 at 1 53 04 pm

(If there is a way for you to share the testnet account for the above transactions, that would help as well).

KayinCheung commented 7 years ago

@sogko

Interesting the garbage characters do not appear on your browser. For reference, I am using google chrome version 60.0.3112.113 with Parity, 3820x2160 screen and 200% scaling. Will test the Dapp on another system tomorrow.

Here are the hex values for status column In Progress: 0x496e2050726f6772657373000000000000000000000000000000000000000000 Complete: 0x436f6d706c657465000000000000000000000000000000000000000000000000

I applied the fixed table width. It does not solve the core issue of garbage characters, but at least it no longer breaks alignment.

garbage char

Table css

table.fixed {table-layout:fixed; width:700px;}
table.fixed td {overflow:visible;}
table.fixed td:nth-of-type(1) {width:45px;}
table.fixed td:nth-of-type(2) {width:335px;}
table.fixed td:nth-of-type(3) {width:160px;}
table.fixed td:nth-of-type(3) {width:160px;}

Also replied to you on reddit, but let's discuss here.

KayinCheung commented 7 years ago

Inspect element of garbage characters. They appear to be white spaces.

inspect_element

sogko commented 7 years ago

Hi @KayinCheung

Also replied to you on reddit, but let's discuss here.

Sure! (Btw, I never did post a reply on reddit; I'm more active on gihthub ;)

I am using google chrome version 60.0.3112.113 with Parity, 3820x2160 screen and 200% scaling.

My system information:

Here are the hex values for status column In Progress: 0x496e2050726f6772657373000000000000000000000000000000000000000000 Complete: 0x436f6d706c657465000000000000000000000000000000000000000000000000

I've inspected the values for statuses on my transaction history and verified that they are the same hex values.

I've digged deeper and it appears toAscii() is the culprit. The hex value input are padded with 0 till 32 bytes, which results in ASCII string with null characters at the end.

I've tested it using the version of web3 that you use (I believe its 0.16, latest release is 0.19).

> web3.toAscii("0x436f6d706c657465000000000000000000000000000000000000000000000000") === "Complete"
> false
> web3.toAscii("0x436f6d706c657465") === "Complete"
> true
> web3.toAscii("0x436f6d706c657465000000000000000000000000000000000000000000000000") === "Complete\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000"
> true

So you have two ways to address this issue:

1) Trim the ASCII string result by removing the null characters at the end:

{this.state.ETH_CLIENT.toAscii(callInfo[3][index]).replace(/\u0000/g, '')}
> web3.toAscii("0x436f6d706c657465000000000000000000000000000000000000000000000000").replace(/\u0000/g, '') === "Complete"
> true

2) Use toUtf8() instead, as it handles strings and better.

> web3.toUtf8("0x436f6d706c657465000000000000000000000000000000000000000000000000") === "Complete"
> true

You can compare the two utils method by taking a look at the tests on ethereum/web3.js, and see how each util handles strings

screen shot 2017-09-15 at 11 18 26 am

Hope this had helped you with your dapp project 👍

KayinCheung commented 7 years ago

@sogko

Sure! (Btw, I never did post a reply on reddit; I'm more active on gihthub ;)

My bad, it was another person.

I tested the below versions locally, with the current Web3 V0.16 and with Web3 V0.20.

{this.state.ETH_CLIENT.toUtf8(callInfo[3][index])} {this.state.ETH_CLIENT.toAscii(callInfo[3][index]).replace(/\u0000/g, '')} {this.state.ETH_CLIENT.toAscii((callInfo[3][index]).replace(/\u0000/g, ''))} {this.state.ETH_CLIENT.toAscii(callInfo[3][index]).replace(/ /g, '')} {this.state.ETH_CLIENT.toAscii((callInfo[3][index]).replace(/ /g, ''))}

Testing was done on 2 computers, but the issue still persists.

Computer 1

Computer 2

Technically the white space exists before next page or previous page is clicked. It might not be a white space issue, but something with changing string lengths.

With fixed table widths, the problem is less severe and I will revisit this issue in the future. Appreciate the time you took to research. There was a public bounty for this issue, let me know your Eth address. :)

sogko commented 7 years ago

Hi @KayinCheung

I've PM'ed you (/u/Arrow222) on reddit with my address (/u/sogko) 👍

Let me see if I can get my hands on a Windows 10 machine. If you need further help to look into this issue and feel comfortable with sharing the React project (or other issues), let me know.

Cheers!

KayinCheung commented 7 years ago

I found a solution to the issue and the fix is live for buyer dashboard.

The problem doesn't happen when data is loaded onto an empty transaction table. So the solution is clearing the populated table first, before repopulating data.

An unintended side effect is, clicking next page/prev page or changing address resets any scroll position to default. Page jumps a little when performing these actions. A future update will shorten the maximum dashboard vertical length to contain it without scrolling for 1920x1080 resolutions and higher, removing page jump issue for most users.

Closing issue as solution is found.

@sogko Please check your address. :)

KayinCheung commented 7 years ago

Upon further digging, there was this error in console "Each child in an array should have a unique "key" prop."

Googling this error brings me to this ReactJs documentation, quote "Keys help React identify which items have changed, are added, or are removed. Keys should be given to the elements inside the array to give the elements a stable identity:"

So this is probably the core issue of garbage characters. The proper solution is to make a unique key for every dynamically generated td element on a page.

KayinCheung commented 7 years ago

Generating unique keys for each TD element solved the problem

Here's the new code for generating transaction tables

getBuyerHistory(e, startID){

    var TableRows = []
    var callInfo
    this.state.mee.buyerHistory(e, startID, function(error, result){
        if(!error)
        {
            callInfo = result
            _.eachRight(callInfo[0], (value, index) => {
            TableRows.push(             
            <tr>

            <td key={String(this.state.fromAddress) + String(startID) + String(index) + "id"}><p className="small">#{this.state.startID + index}</p></td>     
            <td key={String(this.state.fromAddress) + String(startID) + String(index) + "selleraddress"}>Purchase from<p className="small">{callInfo[0][index]}</p></td>                
            <td key={String(this.state.fromAddress) + String(startID) + String(index) + "amount"}>{+(parseFloat(callInfo[2][index])/(10**18)).toFixed(6)} ETH</td>
            <td key={String(this.state.fromAddress) + String(startID) + String(index) + "status"}>{this.state.ETH_CLIENT.toAscii(callInfo[3][index])}</td>              
            </tr>   
            )
        })

        this.setState({tableRows: TableRows});
        }   
        else
            console.log(error)
        }.bind(this))

    }

Keys are generated from combining current selected address, the transaction id number and a column identifier in a long string.

Old code

getBuyerHistory(e, startID){

    var TableRows = []
    var callInfo
    this.state.mee.buyerHistory(e, startID, function(error, result){
        if(!error)
        {

            callInfo = result
            _.eachRight(callInfo[0], (value, index) => {
            TableRows.push(                     
            <tr>

            <td><p className="small">#{this.state.startID + index}</p></td>   
            <td>Purchase from<p className="small">{callInfo[0][index]}</p></td>             
            <td>{+(parseFloat(callInfo[2][index])/(10**18)).toFixed(6)} ETH</td>
            <td>{this.state.ETH_CLIENT.toAscii(callInfo[3][index])}</td>            
            </tr>   
        )
        })

        this.setState({tableRows: TableRows});
        }   
        else
            console.log(error)
        }.bind(this))

    }

Update now published live.