Copy OTP button added to the search results #257
Conversation
Changes to be committed: modified: chrome/background.browserify.js new file: chrome/icon-otp.svg modified: chrome/script.browserify.js modified: chrome/styles.css
chrome/background.browserify.js
Outdated
| @@ -112,6 +112,8 @@ function onMessage(request, sender, sendResponse) { | |||
| text = response.p; | |||
| } else if (request.what === "username") { | |||
| text = response.u; | |||
| } else if (request.what === "otp") { | |||
| text = response.digits; | |||
There was a problem hiding this comment.
I've just realized that a large majority of users don't use OTP in browserpass (I know maybe of 5 people in total), and with this change everyone will see a new button in front of every record that literally does nothing.
Adding it is probably fine, because you could argue "I personally don't use launch URL / copy username buttons, so what", but I think at the very least it should not act as if the copy was successful. If this entry did not have OTP code, instead of silently closing, let's show an error - similar to what happens when you try to launch a URL and the URL is not found.
Something like this:
if (!response.digits) {
sendResponse({
status: "ERROR",
error:
"Unable to determine the OTP code for this entry."
});
return;
}/cc @erayd maybe we could consider making which buttons are visible configurable in v3. Especially if we will be adding new fields (credit card, address, etc - see #187), at some point users will want to have the corresponding buttons, but it doesn't make sense to show them to everyone.
There was a problem hiding this comment.
Fair point. I will add it.
chrome/script.browserify.js
Outdated
| }), | ||
| m("button.copy.otp", { | ||
| onclick: loginToClipboard.bind({ entry: login, what: "otp" }), | ||
| title: "Copy OTP", |
There was a problem hiding this comment.
I'm personally more used to seeing "OTP code" phrase, not sure if it is correct, but if you don't care (😃), could I please have "Copy OTP code" hint here?
There was a problem hiding this comment.
I absolutely don't mind. I am back-end engineer, so historically I don't care about UI :)
|
@maximbaz What if we added a right-click menu, which loaded the file before displaying, so that we know which options are available? We can't decrypt every file to figure out which buttons to display (because decryption is slow), but we can certainly decrypt one. |
|
I like that, noted for v3 😉 |
|
Suggested changes have been implemented. Please review. |
|
My pleasure! |
Changes to be committed:
modified: chrome/background.browserify.js
new file: chrome/icon-otp.svg
modified: chrome/script.browserify.js
modified: chrome/styles.css
Icon is pulled from https://github.com/danklammer/bytesize-icons/blob/master/dist/icons/clock.svg