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

fix: [lw-12182] add autofocus for all password inputs #1698

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vetalcore
Copy link
Contributor

@vetalcore vetalcore commented Feb 7, 2025

Checklist

  • JIRA - LW-12182
  • Proper tests implemented
  • Screenshots added.

Proposed solution

Use custom hook that queries input by id and calls .focus() if found.

Testing

  • sign data in dapp popup
  • sign tx in dapp popup
  • create collateral in dapp popup
  • confirm send tx
  • set collateral in settings
  • sign data in settings
  • show mnemonics in settings
  • set password in paper wallet setup
  • confirm staking (single and multi)
  • enable account drawer
  • create shared wallet setup

nami mode is out of the scope since it was already working there.*

Screenshots

Attach screenshots here if implementation involves some UI changes

@vetalcore vetalcore self-assigned this Feb 7, 2025
@vetalcore vetalcore requested a review from a team as a code owner February 7, 2025 01:15
@pczeglik-iohk
Copy link
Contributor

pczeglik-iohk commented Feb 7, 2025

Allure Report

allure-report-publisher generated test report!

processReports: ✅ test report for 2ebcdebf

passed failed skipped flaky total result
Total 33 0 4 0 37

@vetalcore vetalcore force-pushed the feat/lw-12182-password-inputs-auto-focus branch from 3d8c82b to 3560ce0 Compare February 7, 2025 01:44
Copy link
Contributor

@przemyslaw-wlodek przemyslaw-wlodek left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -84,6 +89,8 @@ export const SignData = (): React.ReactElement => {
onSubmit={(e) => handleSubmit(e, password)}
error={validPassword === false}
errorMessage={t('browserView.transaction.send.error.invalidPassword')}
id={inputId}
autoFocus
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember we had some problems with autoFocus when singing a regular transaction. Not sure if that's the case but maybe hook usage is enough?

Copy link
Contributor Author

@vetalcore vetalcore Feb 7, 2025

Choose a reason for hiding this comment

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

i've tested it locally, seems to be working just fine
let's see what would be the results of sdet review?

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
export const useAutoFocus = (inputRef: any, autoFocus?: boolean): void => {
export const useAutoFocus = <T extends HTMLInputElement | InputRef>(
inputRef: RefObject<T> | string,
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 rename it to inputRefOrId?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done here 8c4ce04

@@ -72,6 +72,7 @@
"react-virtuoso": "^4.6.3",
"recharts": "^2.9.2",
"use-resize-observer": "^9.1.0",
"uuid": "^8.3.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggesting: Can we lift this to the top package.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done here 8c4ce04

@vetalcore vetalcore requested a review from a team as a code owner February 7, 2025 11:11
@vetalcore vetalcore force-pushed the feat/lw-12182-password-inputs-auto-focus branch 2 times, most recently from 8c4ce04 to ad4ab25 Compare February 7, 2025 13:24
@vetalcore vetalcore force-pushed the feat/lw-12182-password-inputs-auto-focus branch from ad4ab25 to 2ebcdeb Compare February 7, 2025 13:29
Copy link

sonarqubecloud bot commented Feb 7, 2025

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