BitBoxSwiss / bitbox-wallet-app

The BitBoxApp for desktop and mobile.
https://bitbox.swiss/app
Apache License 2.0
245 stars 82 forks source link

frontend: transform aopp.tsx to funcitonal component #2761

Open NicolaLS opened 1 month ago

NicolaLS commented 1 month ago

Tested by patching the Start function in backend like this.

I actually forgot to remove the commit, but I guess it is good to leave it in there for reviewers to test the PR. I can remove it in the end. (note the delay is 15 seconds when testing). removed it now.

I tested on webdev firefox with BB02B keystore and servewallet-mainnet. I tested the following case statements form the return: error inactive user-approval awaiting-keystore signing

I think choosing-account and syncing was tested too and looked fine but it was to fast to take a screenshot.

I think I did not test success because I don't know a callback to test aopp with.

Some pictures from testing:

aopp-test4 png aopp-test3 aopp-test2 test-aopp1

NicolaLS commented 1 month ago

@thisconnect thanks :) PTAL, I added a comment regarding the shouldUpdateAccountDefault explaining the reasoning.

NicolaLS commented 4 weeks ago

@thisconnect followed your suggestions and removed the shouldUpdateAccountDefault stuff, and tested again.

NicolaLS commented 1 week ago

not sure why CI fails, running make ci locally succeeds

thisconnect commented 1 week ago

not sure why CI fails, running make ci locally succeeds

not related to your PR, we have a new script that checks for missing placeholders in translations https://github.com/BitBoxSwiss/bitbox-wallet-app/commit/3d34cb6146a36a96c76963b7caaa9bd2019297a5 and also Ci does localize check https://github.com/BitBoxSwiss/bitbox-wallet-app/blob/2a2e2b84325adb8a58a3dc1fa74db158f2305c50/scripts/ci.sh#L35

master was failing for a while as some placeholders in translations were missing, but it should pass since https://github.com/BitBoxSwiss/bitbox-wallet-app/pull/2794

benma commented 1 week ago

@thisconnect actually it was a different error, some transient backend/handler test failed for some reason. Re-run was successful.

thisconnect commented 1 week ago

actually it was a different error, some transient backend/handler test failed for some reason. Re-run was successful.

oups I just assumed and didn't check 😇 sorry