Login with wallet though shell: command format #61
Conversation
bin/near
Outdated
| type: 'string', | ||
| default: 'https://wallet.nearprotocol.com', | ||
| }) | ||
| .option('accountId', { |
There was a problem hiding this comment.
shouldn't it be chosen from wallet UI?
There was a problem hiding this comment.
We need to know accountId on the shell side, so it's either passing accountId to wallet and then failing if it can't be used, or setting up some callback mechanism to shell. 1st option seems much simpler
There was a problem hiding this comment.
1st option isn't good UX for when you don't have any account yet.
It makes sense though as a shorthand to pre-select existing account in wallet.
|
Anyway it makes sense to review them together with this PR.
AFAIK we discussed this as a basic part of UX for the feature. Isn't a blocker for this PR, but is necessary to provide decent UX. |
|
added the prompt to enter account id. callback to shell would be a diff pr |
|
|
||
| exports.login = async function(options) { | ||
| if (!options.walletUrl) { | ||
| console.log("Log in is not needed on this environment. Please use appropriate master account for shell operations.") |
There was a problem hiding this comment.
this is hard to understand for outsider. Probably makes sense to link to docs on how to run local setup.
| newUrl.searchParams.set('title', title); | ||
| const keyPair = await KeyPair.fromRandom('ed25519'); | ||
| newUrl.searchParams.set('public_key', keyPair.getPublicKey()); | ||
| console.log(`Please navigate to this url and follow the insturctions to log in: ${newUrl.toString()}`); |
There was a problem hiding this comment.
s/insturctions/instructions/
probably good idea to put newline before URL to make it easier to copy if not clickable in given terminal
No description provided.