-
Notifications
You must be signed in to change notification settings - Fork 29
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
Skip password when creating a new address #1685
Conversation
<React.Fragment> | ||
<SubviewHeader title="" /> | ||
{!authError && <Loading />} | ||
{authError && <AppError>{authError}</AppError>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the scenarios where this might happen? I think generally it'd be great to give the user a way to fix whatever went wrong. For ex, if we think that this was due to us somehow having an incorrect password in the background
, maybe we should show them the Enter Password
field so they can put the correct password in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@piyalbasu Tbh I'm not sure yet about all the scenarios which could result in an auth error here. I was basically trying to guard against this error since we were already doing it. But I see what you mean, in the old UX the user would always have the chance to input the password again. 🤔
I think I'll make a change so that if there is an auth error the user will be redirected to the "Enter Password" screen with the error message right below the input inbox so we keep the same old UX in this case allowing users to input the password again. What do you think?
Thanks for the review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@piyalbasu on this commit I've changed the error UX to be like the below. So users will always have the chance to input the password and fix the issue themselves:
Screen.Recording.2024-11-19.at.14.46.09.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I believe the only way a user can have authError
is if they specifically enter the wrong password. If they enter the correct password, it should clear this error. But if they don't, it definitely makes sense to show them the input so they have the opportunity to enter the correct password. Thanks for guarding against this scenario!
return ( | ||
<React.Fragment> | ||
<SubviewHeader title="" /> | ||
{!authError && <Loading />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is worth a change or not but just for thought -
We have other places in Freighter that use a similar pattern but also produce the "loading stutter" effect we get when creating an account here. This happens because there are 2 loaders for 1 transition in state.
Is there an easy way to make that feel more like 1 loading state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aristidesstaffieri Hummm I think I know what you mean. I believe there is probably a way of making this transition look nicer but I'm not sure it'll be a straightforward fix.
I'm still getting used to the codebase and to the React Web syntax (which is different from React Native syntax) so I think it'd take some time for me to figure this out. In case you think it's not a blocker, I'd like to move ahead "as is" and create a ticket so we could tackle it later. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review btw!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure thing and yeah I think it's not a blocker, I do wonder if it's better to leave this loading state off for now. It feels a bit less choppy to me and I don't see that we had one in this flow before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aristidesstaffieri on this commit I've removed the duplicate spinner so the UX is looking like the below. I think it's looking less chopy now :)
Screen.Recording.2024-11-19.at.14.33.46.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, thanks for entertaining that. We can think about how to improve the lag between the transition in a follow up.
…while redirecting
* adds size constraints for the favicon container to avoid overflowing icons of non standard sizes (#1673) * [BUG] makes swap amount persist across destination asset changes (#1675) * makes swap amount persist across destination asset changes * updates swap test state to init with balances * adds enough padding to accomodate action modal on scrollable lists (#1676) * Allow creating a new wallet from the `Enter Password` screen (#1648) * Allow creating a new wallet from initial screen * Added translations * Feature/maintain password 2 (#1677) * maintain password when switching between accounts * Added translations * add test and update e2e token * enable e2e tests in CI and add cleanup tasks (#1684) * enable e2e tests in CI and add cleanup tasks * fix asset code change * Redesign Connected apps (#1686) * Added translations * Add 'Disconnect all' button * Added translations * Display favicon * Skip password when creating a new address (#1685) * Skip password when creating new address * Better copy for Enter Password screen * Added translations * Allow user to input password again in case of error + remove spinner while redirecting * limit how many instances of the test can run at a time to prevent collisions (#1690) * Prevent infinity spinner when switching to same account (#1692) * splits buildAndSimulateSoroswapTx into flows for custom networks, uses simulate-tx API for other networks (#1694) * makes error boundary always close window, tweaks css to align at all sizes (#1696) * [REFACTOR] History data fetching (#1698) * adds use-get-history hook, and replaces data fetching for history using it * removes stray log * move loading render branch out of main render to avoid ternary * tweak hasEmptyHistory check to account for an empty operations list * renames use get history hook file to match repo conventions * renames var to follow code convention * redesign the Create New Wallet screens and update tests (#1699) * redesign the Create New Wallet screens and update tests * rm console.logs * fix checks in e2e login helpers * updating recovery snapshot. this screen will be redesigned in next PR * fix incorrect mnemonic phrase test * Redesign Unfunded account notification (#1703) * Redesign Unfunded account notification * Add "Learn more" link on notification body * Feature/redesign onboarding recover wallet (#1704) * redesign recover flow and add tests * Added translations * rm unnecessary pw check in cleanup and increase threshold of 24 word snapshot * fix threshold adjustment * make Toggle component tab-able * use box-shadow to align with Select * updating cleanup tasks in e2e tests * redesign account view + account details QR code (#1707) * redesign account view + account details QR code * apply same styling to network selector dropdown * fix checks for both dropdowns * Charles PR comments * add github actions for deploying @stellar/freighter-api (#1708) * add github actions for deploying @stellar/freighter-api * use yarn to config tag and commit strings * Redesign Login and Re-Auth screens (#1706) * Redesign footer on UnlockAccount * Redesign input and button on UnlockAccount * Redesign text on UnlockAccount * Add identicon UI on UnlockAccount * Add service to set/load last used account * Workaround lint error * UI tweak * Extract reusable EnterPassword component * Use EnterPassword component on AddAccount screen * Use EnterPassword component on VerifyAccount screen * Added translations * Redesign NavBar for UnlockAccount screen * Make 1.5rem padding consistent in all screen contents (Figma) * Use pxToRem() * Hide Identicon on EnterPassword component in case address is not provided * Avoid using same name of the duck function * Update lastUsedAccount on Account screen instead of Sign out action * Change default description for EnterPassword * Added translations * Tweak text size and alignment * Set lastUsedAccount every time user subscribes to an account * Bump the all-actions group across 1 directory with 7 updates (#1709) Bumps the all-actions group with 7 updates in the / directory: | Package | From | To | | --- | --- | --- | | [actions/checkout](https://github.com/actions/checkout) | `2` | `4` | | [actions/setup-node](https://github.com/actions/setup-node) | `1` | `4` | | [jossef/action-set-json-field](https://github.com/jossef/action-set-json-field) | `1` | `2` | | [restackio/update-json-file-action](https://github.com/restackio/update-json-file-action) | `1e34d747ca044df80b37bc4ef5a9aa41be9dac8f` | `f8ef1561cb15ba86a6367b547216375bc60e7f91` | | [rtCamp/action-slack-notify](https://github.com/rtcamp/action-slack-notify) | `2.3.0` | `2.3.2` | | [frabert/replace-string-action](https://github.com/frabert/replace-string-action) | `1.2` | `2.5` | | [ruby/setup-ruby](https://github.com/ruby/setup-ruby) | `1.138.0` | `1.203.0` | Updates `actions/checkout` from 2 to 4 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v2...v4) Updates `actions/setup-node` from 1 to 4 - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@v1...v4) Updates `jossef/action-set-json-field` from 1 to 2 - [Release notes](https://github.com/jossef/action-set-json-field/releases) - [Commits](jossef/action-set-json-field@v1...2a0f7d9) Updates `restackio/update-json-file-action` from 1e34d747ca044df80b37bc4ef5a9aa41be9dac8f to f8ef1561cb15ba86a6367b547216375bc60e7f91 - [Release notes](https://github.com/restackio/update-json-file-action/releases) - [Commits](restackio/update-json-file-action@1e34d74...f8ef156) Updates `rtCamp/action-slack-notify` from 2.3.0 to 2.3.2 - [Release notes](https://github.com/rtcamp/action-slack-notify/releases) - [Commits](rtCamp/action-slack-notify@4e5fb42...c337377) Updates `frabert/replace-string-action` from 1.2 to 2.5 - [Release notes](https://github.com/frabert/replace-string-action/releases) - [Commits](frabert/replace-string-action@v1.2...b6828c5) Updates `ruby/setup-ruby` from 1.138.0 to 1.203.0 - [Release notes](https://github.com/ruby/setup-ruby/releases) - [Changelog](https://github.com/ruby/setup-ruby/blob/master/release.rb) - [Commits](ruby/setup-ruby@v1.138.0...v1.203.0) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-actions - dependency-name: actions/setup-node dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-actions - dependency-name: jossef/action-set-json-field dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-actions - dependency-name: restackio/update-json-file-action dependency-type: direct:production dependency-group: all-actions - dependency-name: rtCamp/action-slack-notify dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-actions - dependency-name: frabert/replace-string-action dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-actions - dependency-name: ruby/setup-ruby dependency-type: direct:production update-type: version-update:semver-minor dependency-group: all-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump the minor-and-patch group across 3 directories with 16 updates (#1710) Bumps the minor-and-patch group with 15 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@babel/preset-react](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-react) | `7.25.9` | `7.26.3` | | [@vercel/ncc](https://github.com/vercel/ncc) | `0.38.2` | `0.38.3` | | [webpack](https://github.com/webpack/webpack) | `5.95.0` | `5.97.1` | | [@blockaid/client](https://github.com/blockaid-official/blockaid-client-node) | `0.28.1` | `0.31.0` | | [@docusaurus/core](https://github.com/facebook/docusaurus/tree/HEAD/packages/docusaurus) | `3.5.2` | `3.6.3` | | [@docusaurus/preset-classic](https://github.com/facebook/docusaurus/tree/HEAD/packages/docusaurus-preset-classic) | `3.5.2` | `3.6.3` | | [@sentry/browser](https://github.com/getsentry/sentry-javascript) | `8.36.0` | `8.42.0` | | @stellar/typescript-wallet-sdk-km | `1.7.0` | `1.8.0` | | [@types/chrome](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/chrome) | `0.0.277` | `0.0.287` | | [sass](https://github.com/sass/dart-sass) | `1.80.5` | `1.82.0` | | [tsconfig-paths-webpack-plugin](https://github.com/dividab/tsconfig-paths-webpack-plugin) | `4.1.0` | `4.2.0` | | [tslib](https://github.com/Microsoft/tslib) | `2.7.0` | `2.8.1` | | [yup](https://github.com/jquense/yup) | `1.4.0` | `1.5.0` | | [@playwright/test](https://github.com/microsoft/playwright) | `1.48.0` | `1.49.0` | | [@sentry/webpack-plugin](https://github.com/getsentry/sentry-javascript-bundler-plugins) | `2.22.6` | `2.22.7` | Bumps the minor-and-patch group with 1 update in the /@shared/api directory: [@blockaid/client](https://github.com/blockaid-official/blockaid-client-node). Bumps the minor-and-patch group with 3 updates in the /extension directory: [@types/chrome](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/chrome), [tslib](https://github.com/Microsoft/tslib) and [@playwright/test](https://github.com/microsoft/playwright). Updates `@babel/preset-react` from 7.25.9 to 7.26.3 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.3/packages/babel-preset-react) Updates `@vercel/ncc` from 0.38.2 to 0.38.3 - [Release notes](https://github.com/vercel/ncc/releases) - [Commits](vercel/ncc@0.38.2...0.38.3) Updates `webpack` from 5.95.0 to 5.97.1 - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.95.0...v5.97.1) Updates `@blockaid/client` from 0.28.1 to 0.31.0 - [Release notes](https://github.com/blockaid-official/blockaid-client-node/releases) - [Changelog](https://github.com/blockaid-official/blockaid-client-node/blob/main/CHANGELOG.md) - [Commits](blockaid-official/blockaid-client-node@v0.28.1...v0.31.0) Updates `@docusaurus/core` from 3.5.2 to 3.6.3 - [Release notes](https://github.com/facebook/docusaurus/releases) - [Changelog](https://github.com/facebook/docusaurus/blob/main/CHANGELOG.md) - [Commits](https://github.com/facebook/docusaurus/commits/v3.6.3/packages/docusaurus) Updates `@docusaurus/preset-classic` from 3.5.2 to 3.6.3 - [Release notes](https://github.com/facebook/docusaurus/releases) - [Changelog](https://github.com/facebook/docusaurus/blob/main/CHANGELOG.md) - [Commits](https://github.com/facebook/docusaurus/commits/v3.6.3/packages/docusaurus-preset-classic) Updates `@sentry/browser` from 8.36.0 to 8.42.0 - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/develop/CHANGELOG.md) - [Commits](getsentry/sentry-javascript@8.36.0...8.42.0) Updates `@stellar/typescript-wallet-sdk-km` from 1.7.0 to 1.8.0 Updates `@types/chrome` from 0.0.277 to 0.0.287 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/chrome) Updates `mini-css-extract-plugin` from 2.9.1 to 2.9.2 - [Release notes](https://github.com/webpack-contrib/mini-css-extract-plugin/releases) - [Changelog](https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/CHANGELOG.md) - [Commits](webpack-contrib/mini-css-extract-plugin@v2.9.1...v2.9.2) Updates `sass` from 1.80.5 to 1.82.0 - [Release notes](https://github.com/sass/dart-sass/releases) - [Changelog](https://github.com/sass/dart-sass/blob/main/CHANGELOG.md) - [Commits](sass/dart-sass@1.80.5...1.82.0) Updates `tsconfig-paths-webpack-plugin` from 4.1.0 to 4.2.0 - [Changelog](https://github.com/dividab/tsconfig-paths-webpack-plugin/blob/master/CHANGELOG.md) - [Commits](dividab/tsconfig-paths-webpack-plugin@v4.1.0...v4.2.0) Updates `tslib` from 2.7.0 to 2.8.1 - [Release notes](https://github.com/Microsoft/tslib/releases) - [Commits](microsoft/tslib@v2.7.0...v2.8.1) Updates `yup` from 1.4.0 to 1.5.0 - [Release notes](https://github.com/jquense/yup/releases) - [Changelog](https://github.com/jquense/yup/blob/master/CHANGELOG.md) - [Commits](jquense/yup@v1.4.0...v1.5.0) Updates `@playwright/test` from 1.48.0 to 1.49.0 - [Release notes](https://github.com/microsoft/playwright/releases) - [Commits](microsoft/playwright@v1.48.0...v1.49.0) Updates `@sentry/webpack-plugin` from 2.22.6 to 2.22.7 - [Release notes](https://github.com/getsentry/sentry-javascript-bundler-plugins/releases) - [Changelog](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/main/CHANGELOG.md) - [Commits](getsentry/sentry-javascript-bundler-plugins@2.22.6...2.22.7) Updates `@blockaid/client` from 0.28.1 to 0.31.0 - [Release notes](https://github.com/blockaid-official/blockaid-client-node/releases) - [Changelog](https://github.com/blockaid-official/blockaid-client-node/blob/main/CHANGELOG.md) - [Commits](blockaid-official/blockaid-client-node@v0.28.1...v0.31.0) Updates `@types/chrome` from 0.0.277 to 0.0.287 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/chrome) Updates `tslib` from 2.7.0 to 2.8.1 - [Release notes](https://github.com/Microsoft/tslib/releases) - [Commits](microsoft/tslib@v2.7.0...v2.8.1) Updates `@playwright/test` from 1.48.0 to 1.49.0 - [Release notes](https://github.com/microsoft/playwright/releases) - [Commits](microsoft/playwright@v1.48.0...v1.49.0) --- updated-dependencies: - dependency-name: "@babel/preset-react" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@vercel/ncc" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-and-patch - dependency-name: webpack dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@blockaid/client" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@docusaurus/core" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@docusaurus/preset-classic" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@sentry/browser" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@stellar/typescript-wallet-sdk-km" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@types/chrome" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-and-patch - dependency-name: mini-css-extract-plugin dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-and-patch - dependency-name: sass dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: tsconfig-paths-webpack-plugin dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: tslib dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: yup dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@playwright/test" dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@sentry/webpack-plugin" dependency-type: direct:development update-type: version-update:semver-patch dependency-group: minor-and-patch - dependency-name: "@blockaid/client" dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@types/chrome" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-and-patch - dependency-name: tslib dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-and-patch - dependency-name: "@playwright/test" dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-and-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * allow signing auth entries using Ledger (#1723) * allow signing auth entries using Ledger * rename handleSignedHwTx to reflect that it can be either a tx or a signed auth * commit new lockfile to align package file and lock file (#1731) * Feature/rm blockaid modal 2 (#1732) * remove blockaid announcement modal * Added translations * update e2e test token to newly deployed token (#1724) * update e2e test token to newly deployed token * fixing typo in test name * match bottom nav more closely to designs (#1734) * Only render Account footer when needed (#1733) * fix string interpolation in beta slack msg (#1735) * update display mnemonic phrase screen (#1738) * update display mnemonic phrase screen * try uploading test-results artifacts * update retention time and update snapshot * Bugfix/5.26.0 qa (#1739) * QA fixes for 5.26.0 * update snaphost and add defaults for verified token * updating 24 word mnemonic snapshot * mask mnemonic phrase wrapper * skip this test since our test token is currently verified (#1744) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Cássio Marcos Goulart <[email protected]> Co-authored-by: Piyal Basu <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Closes #923
This PR implements the following changes as discussed on the issue:
password is STILL saved
in current session store users won't need to input their password when creating a new Stellar address. Instead, the application will automatically read it from the session state.password is NOT saved
(e.g. the session timed out) then it'll ask for the password.couple copies
on this "Enter Password" screen:Creating a new address when password is saved
Screen.Recording.2024-11-19.at.14.33.46.mov
Creating a new address when password is NOT saved
Screen.Recording.2024-11-18.at.11.40.12.mov
Auth error while creating a new address
Screen.Recording.2024-11-19.at.14.46.09.mov