Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding selection field for keypair type #1709

Merged
merged 8 commits into from
Dec 20, 2023

Conversation

maayarosama
Copy link
Contributor

@maayarosama maayarosama commented Dec 17, 2023

Description

Adding selection for key pair type in profile manager
Screenshot from 2023-12-18 16-12-50

Changes

  • Added selection for key pair type
  • Added a warning for the user
  • Added key pair type in profile interface
  • Updated get grid method

Related Issues

#1113

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

Copy link
Contributor

@AhmedHanafy725 AhmedHanafy725 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to re-validate the mnemonic if the keypair is changed as most probably the account will be created with a certain keypair type

@@ -638,6 +659,7 @@ async function createNewAccount() {
creatingAccount.value = true;
try {
const account = await createAccount();
console.log("account", account);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the debug logs

@@ -710,6 +732,7 @@ profileManagerController.set({ loadBalance: __loadBalance });

function login() {
const credentials: Credentials = getCredentials();
console.log(credentials);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too

@AlaaElattar
Copy link
Contributor

  • Validation should be reloaded.
  • In that case, I've added mnemonic of type ed25519 and then chose it.
Screencast.from.12-19-2023.03.00.31.PM.webm

@AhmedHanafy725
Copy link
Contributor

not sure if we should move the new warning message to the top or keep it. But if we keep it, I think it should have some spacing after @ehab-hassan

image

@@ -252,6 +267,9 @@
<v-alert type="error" variant="tonal" class="mt-2 mb-4" v-if="loginError">
{{ loginError }}
</v-alert>
<v-alert variant="tonal" type="warning" v-if="activeTab === 1">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels more like an info not a warning , what do u think ?

@AlaaElattar
Copy link
Contributor

Also as long as the user can't create account with ed25519 (just import pre existing account), I think w should say that somewhere.

@ehab-hassan
Copy link
Contributor

not sure if we should move the new warning message to the top or keep it. But if we keep it, I think it should have some spacing after @ehab-hassan

image

yes please let's keep it down and make sure to have the same margin's first warning message had.

}
},
{ deep: false },
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we please update the message on line 567 to "Couldn't get the user twin for the provided mnemonic and keytype".

@AhmedHanafy725 AhmedHanafy725 merged commit dee19f8 into development Dec 20, 2023
3 checks passed
@AhmedHanafy725 AhmedHanafy725 deleted the development_hex_seed_profile_manager branch December 20, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants