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

Add a Paste handler for the TOTP interface. #154

Merged
merged 6 commits into from
May 23, 2023
Merged

Conversation

dd32
Copy link
Member

@dd32 dd32 commented May 8, 2023

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..

@dd32 dd32 requested a review from adamwoodnz May 8, 2023 07:05
Copy link
Contributor

@adamwoodnz adamwoodnz left a 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.

settings/src/components/auto-tabbing-input.js Outdated Show resolved Hide resolved
@adamwoodnz
Copy link
Contributor

Happy to work on this if it would take a load off you @dd32

@dd32
Copy link
Member Author

dd32 commented May 10, 2023

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 WIP if I'm still working on it :)

@adamwoodnz adamwoodnz added this to the Iteration 1 milestone May 10, 2023
@adamwoodnz adamwoodnz force-pushed the add/paste-in-totp branch from 16be31d to 8cf7322 Compare May 15, 2023 02:21
// 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;
Copy link
Contributor

@adamwoodnz adamwoodnz May 15, 2023

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

@adamwoodnz adamwoodnz force-pushed the add/paste-in-totp branch 2 times, most recently from 86634ea to 29bddfd Compare May 15, 2023 04:18
@adamwoodnz adamwoodnz requested a review from iandunn May 15, 2023 04:20
@adamwoodnz
Copy link
Contributor

Desktop UI with:

  1. Type incorrect totp, submit
  2. Type change but still incorrect, submit
  3. Paste totp too short over existing (no inputs change, error shown)
  4. Clear and paste totp too short (no inputs change, error shown)
  5. Paste totp correct length, submit
  6. Final correct totp

paste totp

Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@adamwoodnz adamwoodnz force-pushed the add/paste-in-totp branch from aa2afe3 to 345c6c4 Compare May 23, 2023 00:21
@adamwoodnz adamwoodnz merged commit 0d057db into trunk May 23, 2023
@adamwoodnz adamwoodnz deleted the add/paste-in-totp branch May 23, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Can't paste TOTP codes
3 participants