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

Cycle through every login when using the auto-fill shortcut #956

Merged
merged 5 commits into from Aug 12, 2020

Conversation

xusoo
Copy link
Contributor

@xusoo xusoo commented Jun 22, 2019

I wanted to contribute with something to the project (besides the premium subscription) and I saw this open feature request so I decided to implement it myself 😄

Let me know any doubt, any problem or anything you see that could've been done in a better way 👍

Copy link
Member

@kspearrin kspearrin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Please be aware of changes being made that are not necessary (formatting, imports being moved around, etc). This creates a lot of noise that makes it difficult to see what really has changed.

src/services/autofill.service.ts Outdated Show resolved Hide resolved
src/services/autofill.service.ts Outdated Show resolved Hide resolved
src/background/runtime.background.ts Outdated Show resolved Hide resolved
src/background/runtime.background.ts Outdated Show resolved Hide resolved
src/services/autofill.service.ts Show resolved Hide resolved
@xusoo
Copy link
Contributor Author

xusoo commented Jul 2, 2019

Hi Kyle, can you answer my comments here? 🙂

@kspearrin
Copy link
Member

@xusoo I have this on my backlog and will review when I get some time. Sorry for the delay.

@xusoo
Copy link
Contributor Author

xusoo commented Jul 5, 2019 via email

@RayBB
Copy link

RayBB commented Aug 30, 2019

This would be a great feature! @xusoo is there anything you need help with or are we just waiting on review from @kspearrin ? I'm more than happy to help (I've made a few extensions before) if there is anything else to do here.

@MattKang
Copy link

Would just like to second the comments here @kspearrin offering help on this PR and that this would be an incredibly useful feature.

In the process of switching over form 1Password, because I believe this has the potential to be so much better with community support, e.g. this PR

@AriZoNaiCe
Copy link

Would love to see this -- in fact, any sort of "default" instead of just last used would be helpful, too.

@luckman212
Copy link

@xusoo did you ever hear back? @kspearrin have you had a chance to review? This is a highly coveted feature

@xusoo
Copy link
Contributor Author

xusoo commented Mar 4, 2020

No @luckman212 I didn’t hear back 😅

@CLAassistant
Copy link

CLAassistant commented Mar 7, 2020

CLA assistant check
All committers have signed the CLA.

@AidasK
Copy link

AidasK commented Mar 25, 2020

Would love to see this feature released!

@ghost
Copy link

ghost commented May 26, 2020

what's preventing this from being being added?

is it just the cla that needs signing? @xusoo?

other than no soft delete, this one feature is preventing me from switching from 1password

@xusoo
Copy link
Contributor Author

xusoo commented May 26, 2020

what's preventing this from being being added?

is it just the cla that needs signing? @xusoo?

other than no soft delete, this one feature is preventing me from switching from 1password

Oh, I signed it a while ago, I don't know what happened. I've signed it again anyway.

But no, I don't think this was what kept this from being merged. There are still some open questions to @kspearrin from a year ago, I don't really know if I should make some changes to my code or what.

By the way: https://github.com/bitwarden/browser/releases/tag/v1.44.0 (Soft delete)

@ghost
Copy link

ghost commented May 26, 2020

Oh thanks, didn't realise soft delete was added already

Is this still on your radar @kspearrin ?

@AndyDeNike
Copy link

this is an absolutely crucial feature that most mainstream password managers have integrated! Please make this happen :D

@MattKang
Copy link

@kspearrin just wanted to flag this again and volunteer to help with any work required to close out this PR. Can you comment on what still needs to be done? Thanks

@cscharf
Copy link
Contributor

cscharf commented Aug 4, 2020

This feels pretty close folks, just a few tweaks and polyfill/backfill for Safari and we should be able to accept the PR.

@AriZoNaiCe
Copy link

This feels pretty close folks, just a few tweaks and polyfill/backfill for Safari and we should be able to accept the PR.

This would be amazing -- thank you for the continued efforts!

src/services/autofill.service.ts Outdated Show resolved Hide resolved
xusoo pushed a commit to xusoo/jslib that referenced this pull request Aug 6, 2020
To be used from browser extension when autofilling.
Related PR: bitwarden/clients#956
Copy link
Contributor

@cscharf cscharf left a comment

Choose a reason for hiding this comment

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

These look good, I'll approve but still need @kspearrin to clear final changes. Will need to wait for jslib updates before merging in/finalizing.

@cscharf cscharf requested a review from kspearrin August 6, 2020 16:12
xusoo added a commit to xusoo/jslib that referenced this pull request Aug 10, 2020
To be used from browser extension when autofilling.
Related PR: bitwarden/clients#956
@ghost
Copy link

ghost commented Aug 12, 2020

Thanks for keeping this one alive, love your work

kspearrin pushed a commit to bitwarden/jslib that referenced this pull request Aug 12, 2020
* Add new method for cycling through every login

To be used from browser extension when autofilling.
Related PR: bitwarden/clients#956

* Cache sorted ciphers by URL and invalidate them after a period of 5 seconds

* Move file to models
@kspearrin kspearrin merged commit fe2557e into bitwarden:master Aug 12, 2020
@luckman212
Copy link

@kspearrin can't wait to have this available in the Chrome extension! Any idea when you'll push a new release out?

@cscharf
Copy link
Contributor

cscharf commented Aug 22, 2020

Any idea when you'll push a new release out?

@luckman212 , our next major release will likely be coming in a matter of weeks, definitely in September.

@luckman212
Copy link

It's out! This is wonderful, thank you @xusoo, @kspearrin & @cscharf ❤️

@AidasK
Copy link

AidasK commented Sep 16, 2020

Is it in 1.46.1? cmd+shift+l does not seem to cycle

@luckman212
Copy link

Yes it's in 1.46.1. I have my hotkey set to Cmd+S - it does cycle matching logins for me (macOS/Chrome85)
image

@AidasK
Copy link

AidasK commented Sep 16, 2020

@luckman212 Thanks, I have tested on the other website and it works! It does not on the other one, but sadly I can't share a link to it

@kriswilk
Copy link

Great to see this released. However, I've had trouble with it on a few sites and hence raised issue #1392. I'm seeing a number of sites where the cycling gets stuck bouncing between 2 out of my many logins for a given site rather than going through them all.

@luckman212
Copy link

While this is a great feature, it could be a LOT better if 2 things were implemented:

  1. option to cycle alphabetically instead of by MRU
  2. a hotkey to cycle "backwards" so if we go too far we can back up without taking hands off the keyboard

@kriswilk
Copy link

I like both of those ideas. In fact, an argument could be made to make alpha order the default, with the first autofill being the most-recently-used and cycling alpha order after that.

I suggest you create a new issue so that it's not buried in a merged PR.

@luckman212
Copy link

Thanks @kriswilk -- there's already an FR over here https://community.bitwarden.com/t/cycle-through-logins-alphabetically-browser-extension/28478

And, seems we can't open new FRs under GH Issues, it just forwards over to the Community Forums. Not sure how to call more attention to it.

@kriswilk
Copy link

Well, I added my vote on the FR!

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.

None yet