Account selection when authorizing a website

[Tbaut]

closes #1037

• account selection
• un/toggle all accounts
• show a dedicated message when no account was found: screenshot
• refactor the website management screen to allow changing the connected accounts

It looks like this:

pjs-auth.mp4

website-management.mp4

TODO:

• Show helper text & button when the extension doesn't have any account yet.
• Fix the FIXME
• Fix/Add tests
• Add account management per website from the "Manage Website Access" screen.
• Testing on FF and with less accounts
• When an account is forgotten, we need to go through the list of authorized website and remove the said account.

[lgtm-com]

This pull request introduces 1 alert when merging 671a95b into 885a67e - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[Tbaut]

I'm on the fence regarding an edge case:

• you have an account that is set as visible (eye open)
• you select it to connect to a website
  -> the dapp has access to it
• now you set it to invisible (eye closes)
  -> the dapp doesn't have access to it, as expected
  -> now because it's set as invisible, this account isn't listed in the accounts management any more, so that you can't actually remove it from the authorized accounts. Also the UI is inconsistent
  example screenshot

This contradicts what we discussed earlier here. I'm leaning toward still showing the hidden accounts in the AccountSelection, and show their global visibility status. What do you think?

edit: I went ahead and implement what makes the most sense to me, so that we don't have any inconsistent UI, but also hide the accounts with a closed eye as much as possible.

TL;DR:

• on the Auth screen hidden accounts aren't visible, there's now a test for this.
• on the Account management screen they are visible, along with the eye open/close action.

[Tbaut]

I'm pretty sure you've seen this @jacogr, but just in case pinging you here as it's ready to review.

[jacogr]

Sorry, just wanted to get a release out (which overran by 10 days), before merging more code. Since we have https://github.com/polkadot-js/extension/releases/tag/v0.44.1, will take a peek as soon as I'm certain it is on the stores.

[Tbaut]

No problem at all, 100% what I thought, better get this tested well rather than rushed before a release 👍

[jacogr]

Ok - 0.44.1 is on all stores, now back to normality. Will get to this soon.

[jonasW3F]

Looking forward to see this implemented! Is there any ETA on when this ships?

[jacogr]

Is it possible to merge master? On my side I get build failures atm, e.g

    ./src/page.ts + 31 modules 29.6 KiB [built] [code generated]
  crypto (ignored) 15 bytes [built] [code generated]
  crypto (ignored) 15 bytes [built] [code generated]
  buffer (ignored) 15 bytes [optional] [built] [code generated]
  crypto (ignored) 15 bytes [built] [code generated]
webpack 5.73.0 compiled successfully in 4097 ms

*** @polkadot/extension-base 0.44.2-4
Successfully compiled 31 files with Babel (520ms).
Successfully compiled 31 files with Babel (143ms).
TypeError: Cannot read properties of undefined (reading 'startsWith')
    at relativePath (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:70:19)
    at createMapEntry (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:75:12)
    at buildExports (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:228:23)
    at file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:614:50
    at timeIt (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:565:3)
    at buildJs (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:614:7)
    at async main (file:///Users/jacogr/Projects/polkadotjs/test/tbaut-ext/node_modules/@polkadot/dev/scripts/polkadot-dev-build-ts.mjs:661:5)

I think this is maybe something to do with versions?

[Tbaut]

sure I'll check it out

[Tbaut]

I was able to build it without issue. lmk if you still run into troubles.

[jacogr]

Yes, for me - master builds, this branch has issues. It seems to be caused by the dev scripts and have no idea why. (No changes here would cause anything weird). O, god, seems to be one of my issues caused by something I did somewhere, but has me stumped. (Whacked node_ modules and have the same issue)

[jacogr]

Ok, this is not your issue.... local problem :( fuckit.

[jacogr]

It seems fine. I played with this over the last couple of days.

I'm not 100% convinced around the visible/invisible handling - I would probably have gone for the (more complicated option) or deleting it from all, like we handle deletes, when made invisible. But is is certainly an edge-case, as indicated.

[polkadot-js-bot]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.