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

Refactor Connect wallet #3827

Draft
wants to merge 32 commits into
base: development
Choose a base branch
from
Draft

Conversation

0oM4R
Copy link
Contributor

@0oM4R 0oM4R commented Jan 20, 2025

Description

Refactor login and connect wallet

Changes

  • Profile manager:
    • move the logic of connect and login to separate components and use them in Dtab

    • remove all the conditional elements that rely on active tab

  • Env and config.js: use WalletKey from env to make sure that is always consistent on all places
  • gather all the postLogin needed logic into a util function to be reusable
  • create components for the moved logic form profile manager
  • email will be disabled untill the user entered a valid mnemonic to avoid overwriting the entered
  • enable reload mnemonic validation only if there is an issue with validating a mnemonic:
    this means if the entered mnemonic is valid but we couldn't load the grid except TwinNotExistError something like connection error, then we enable the reload

Related Issues

Tested Scenarios

  • in incognato mode tabs works fine
  • create account works fine
  • activate account with sr25519 works fine
  • activate accout with ed25519 will show an error
  • auto login on refresh
  • email loading and storing works fine
  • tabs switching is functional

Documentation PR

For UI changes, Please provide the Documentation PR on info_grid

To consider

Preliminary Checks:

  • Preliminary Checks
    • Does it completely address the issue linked?
    • What about edge cases?
    • Does it meet the specified acceptance criteria?
    • Are there any unintended side effects?
    • Does the PR adhere to the team's coding conventions, style guides, and best practices?
    • Does it integrate well with existing features?
    • Does it impact the overall performance of the application?
    • Are there any bottlenecks or slowdowns?
    • Has it been optimized for efficiency?
    • Has it been adequately tested with unit, integration, and end-to-end tests?
    • Are there any known defects or issues?
    • Is the code well-documented?
    • Are changes to documentation reflected in the code?

UI Checks:

  • UI Checks
    • If a UI design is provided/ does it follow it?
    • Does every button work?
    • Is the data displayed logical? Is it what you expected?
    • Does every validation work?
    • Does this interface feel intuitive?
    • What about slow network? Offline?
    • What if the gridproxy/graphql/chain is failing?
    • What would a first time user have a hard time navigating here?

Code Quality Checks:

  • Code Quality Checks
    • Code formatted/linted? Are there unused imports? .. etc
    • Is there redundant/repeated code?
    • Are there conditionals that are always true or always false?
    • Can we write this more concisely?
    • Can we reuse this code? If yes, where?
    • Will the changes be easy to maintain and update in the future?
    • Can this code become too complex to understand for other devs?
    • Can this code cause future integration problems?

Testing Checklist

  • Does it Meet the specified acceptance criteria.
  • Test if changes can affect any other functionality.
  • Tested with unit, integration, and end-to-end tests.
  • Tested the un-happy path and rollback scenarios.
  • Documentation updated to meet the latest changes.
  • Check automated tests working successfully with the changes.
  • Can be covered by automated tests.

General Checklist

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

0oM4R added 18 commits January 16, 2025 16:20
use wallet key from window.env
create the post login logic
- add form to the login, this will handle the stats tabs more effictive,
- remove the emits that change the field status not needed
add connect wallet
… before mnemonics to avoid conflicts and losing entred data
- getting a grid checking a twin should be in validate mnemonic, moved and marked as asyncRule
- load email only getting email and set it
- also the reload validation now only if there is an issue with getting the grid, in validate mnemonic
- getting a grid checking a twin should be in validate mnemonic, moved and marked as asyncRule
- load email only getting email and set it
- also the reload validation now only if there is an issue with getting the grid, in validate mnemonic
- passing the mnemonic or hex to profile
- disable password on logning
- add emit to update loading
support create and activate account flow
- enhance getting credentials logic
- enhance getting credentials logic
- disable auto complete
- set the session password in handlePostLogin
@0oM4R 0oM4R changed the title Development rewrite profile manager Refactor Connect wallet Jan 29, 2025
@ramezsaeed
Copy link
Contributor

I think this needs docs to be promoted first before we merge it.

@ramezsaeed
Copy link
Contributor

Test scenarios to Add:

  • Test the new login/create account with KYC
  • Test login with Hex (valid & invalid Hex)
  • Test login with mnemonic 24 words
  • Test login with invalid mnemonic
  • Test valid mnemonic and update the email & passwd

@0oM4R
Copy link
Contributor Author

0oM4R commented Jan 30, 2025

I think this needs docs to be promoted first before we merge it.

there is no changes in the UI or the current flow, we just refactored the current code

Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

i get this massive error when i reload the your farms page or when I even just go to the home page
image
image

@0oM4R 0oM4R requested a review from amiraabouhadid February 2, 2025 10:04
@amiraabouhadid
Copy link
Contributor

amiraabouhadid commented Feb 3, 2025

please disable password autofill for the mnemonic field in register tab
image
it gets autofilled whenever I autofill the email and the newly generated mnemonics get lost
image

Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

this validation should not pass
image

Copy link
Contributor

@amiraabouhadid amiraabouhadid left a comment

Choose a reason for hiding this comment

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

tested:

  • creating new account, works
  • importing account, works
  • logging in, works
  • clearing local storage and getting only the register tab, works

@amiraabouhadid
Copy link
Contributor

not sure if this console err is related
Screenshot from 2025-02-03 11-08-29

@amiraabouhadid
Copy link
Contributor

not sure if it is wise to use suspense as it is still an experimental feature that may not be added to the API
image

@0oM4R
Copy link
Contributor Author

0oM4R commented Feb 3, 2025

not sure if it is wise to use suspense as it is still an experimental feature that may not be added to the API image

removed on ef37dee will update the pr body

@0oM4R
Copy link
Contributor Author

0oM4R commented Feb 3, 2025

please disable password autofill for the mnemonic field in register tab image it gets autofilled whenever I autofill the email and the newly generated mnemonics get lost image

those are saved by you; you can manage them in your browser settings; i think it will never get autofilled if you didn't click on it right?

@amiraabouhadid
Copy link
Contributor

please disable password autofill for the mnemonic field in register tab image it gets autofilled whenever I autofill the email and the newly generated mnemonics get lost image

those are saved by you; you can manage them in your browser settings; i think it will never get autofilled if you didn't click on it right?

They won't be autofilled if I don't select to autofill the email, yes but would nice to disable it for mnemonics as it is not a password

@0oM4R 0oM4R marked this pull request as draft February 3, 2025 13:22
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.

3 participants