-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add a Paste handler for the TOTP interface. #154
Conversation
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.
Works for me. I tested longer and shorter strings and I felt like the validation should result in a message for the user.
Happy to work on this if it would take a load off you @dd32 |
@adamwoodnz In general, please feel free to pick up anything I PR (or push extra commits to, to fix something) especially if it's UI/JS related. I'll normally leave things as a Draft or explicitly mention it's a |
16be31d
to
8cf7322
Compare
// Clean input | ||
if ( value.trim() === '' ) { | ||
event.target.value = ''; | ||
value = ''; | ||
} | ||
|
||
newInputs[ index ] = value; | ||
|
||
// Check if all inputs are filled | ||
const allFilled = newInputs.every( ( input ) => '' !== input ); | ||
if ( allFilled && onComplete ) { | ||
onComplete( true ); | ||
} else { | ||
onComplete( false ); | ||
} | ||
newInputs[ index ] = value.trim() === '' ? '' : value; |
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.
Rather than modifying the value of the DOM element here, we can simply update the inputs
, and the data will flow down to the DOM.
The complete
state can also be derived from the inputs
values, rather than checking on every change to an input, see the check for every input having a value here
86634ea
to
29bddfd
Compare
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.
LGTM 👍🏻
29bddfd
to
aa2afe3
Compare
Instead of when one is empty
aa2afe3
to
345c6c4
Compare
Fixes #151
Screen.Recording.2023-05-08.at.5.02.23.pm.mov
The addition of the
onComplete()
is to trigger the Enable button to enable. I've just noticed as reviewing the video above that the clear button leaves Enable button enabled..