Preferences : Payments : Create wallet control text and status flickers when disabling then enabling brave payments.

[alexwykoff]

Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
After creating a wallet, the 'create wallet' button text changes to 'add funds'. If I toggle the 'Enable' control, the text on the control and status flickers when the wallet is re-enabled.

Expected behavior:
The text should remain 'add funds' and neither the control or status text should flicker.

• Platform (Win7, 8, 10? macOS? Linux distro?):
  Tested on OS X
• Brave Version:
  0.11.6 pre-beta3
• Steps to reproduce:
  1. Enable the Brave wallet.
  2. Create a Brave wallet, observe the text on the create wallet control text.
  3. Disable the wallet, observe the text on the add funds control.
  4. Enable the wallet, observe the text on the create wallet/add funds control.
• Screenshot if needed:
  
• Any related issues:

[bridiver]

polish? Is that related?

[bridiver]

reopen after reverting commit

[alexwykoff]

has this been unreverted? in the latest master it appears to be functioning appropriately.

[mrose17]

it was reverted. i think the issue is timing. the fix that got reverted "could not work"... i will consult with @diracdeltas on what can be done.

[bridiver]

the commit addressed the flickering during wallet creation and correctly transitioned from "create wallet" to "creating wallet..." to "add funds", but is there another problem as well? Your screenshot does not look correct because it stays on "create wallet" after clicking

[diracdeltas]

Just to clarify, there are two separate (but probably related) issues that got merged into this one. The one in Alex's first screenshot (wallet button flicker on creation) i could not repro. The one in Alex's second screenshot, which is the one @bridiver fixed and then reverted, happens consistently. I'm leaving this issue open to address the latter.

[bridiver]

the behavior is actually still inconsistent with my change. Sometimes it works correctly and sometimes it doesn't

[mrose17]

yes, that is my point: it's timing related. it may look right once, or twice in a row, but it isn't fixed.

fortunately, @diracdeltas just took a look at something and explained to me what is likely going on, and i think i have a fix.

[diracdeltas]

@bridiver's fix is correct and works with #3430 applied. i can't tell why updateLedgerInfo requires client to be non-null, but it seems that this check should not inupdateLedgerInfo itself to avoid confusion.

[mrose17]

if the fix is correct when something else is applied, then it isn't correct if that something else isn't applied and this fix (#3376) predates that other fix (#3430)

[mrose17]

with respect to the test in updateLedgerInfo ... if the code where organized differently, then we wouldn't have that requirement there, but the code isn't organized differently...