Rebase

In D17549#399471, @bytesofman wrote:

In D17549#399468, @Fabien wrote:

So before this patch it's broken and after this patch it's not, great.
May I ask how it's broken and how it's fixed ? Because I can't spot the change in behavior.

Yes, some history changed before I got this diff to have tests passing. When I first pushed it up, the only test that changed was the App test.

Before this diff

• Cashtab intended behavior is for a spinner to lock the UI when loading is true. This happens on the App component, which renders the routes, so it will work on any given screen.
• However the conditional rendering has a bug, such that even when loading is true, the Spinner may not render. This diff patches the bug.
• For the test in App -- we show that this spinner is in the screen when the component loads, which is expected behavior.

However, there are a number of knock ons from this change. Cashtab has many tests that need to wait for stuff to load. Thish stuff takes longer depending on the screen. Some of the tests are configured to wait for inline spinners to go away, others are configured to wait for cashtab's bootup to complete. Now that the wallet update spinner is fixed, there is something else that some tests may need to wait for. So, this has caused some other tests to fail. Running the CI and locally revealed some tests to be flaky, i.e. it can be difficult to tell which loading component we must wait for, and I have not (yet) managed to come up with a "catch all" routine to just wait for everything to load in a way that is standard for all tests.

At the moment, I don't think I can confidently predict exactly how to fix all of the various tests and their loading conditions to wait for the right spinners to be present or not present. So we may see some test flakiness after this diff (we had test flakiness of the same kind before this diff, it's just that I iterated through it on those tests with this bug in the system; now that the bug is gone there may be some new loading conditions to handle in existing tests).

This bug was discovered while fixing the wallet changing behavior when I noticed that the spinnner was not appearing even when I manually setLoading(true) to wait for a wallet to load.

In D17549#399466, @emack wrote:

I assume the flakiness of the test you're referring to is when the int test is waiting for the spinner to disappear but it doesn't, and the await waitFor() call just hangs there holding up the entire test suite?

// Wait for Cashtab wallet info to load
await waitFor(() =>
    expect(screen.queryByTitle('Loading...')).not.toBeInTheDocument(),
);

In D17549#399468, @Fabien wrote:

So before this patch it's broken and after this patch it's not, great.
May I ask how it's broken and how it's fixed ? Because I can't spot the change in behavior.

So before this patch it's broken and after this patch it's not, great.
May I ask how it's broken and how it's fixed ? Because I can't spot the change in behavior.

I assume the flakiness of the test you're referring to is when the int test is waiting for the spinner to disappear but it doesn't, and the await waitFor() call just hangs there holding up the entire test suite?

tested with a Trezor T

Remove errant newline

Use assert_equal

Less code duplication

feedback

In D17540#399207, @emack wrote:

Actually, I did an actual check on the bundle size, one meg over 227 megs is negligible in the grand scheme of things. Non-issue unless the whole bundle is getting to a point where it may start to affect performance?

Minor nit but tested all ok. Can confirm the transparency is not an issue on mobile screenshots, so must be Brave specific.