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

Configure Default Options for Add/Edit Credentials Page #307

Open
patgmiller opened this issue Oct 11, 2022 · 8 comments
Open

Configure Default Options for Add/Edit Credentials Page #307

patgmiller opened this issue Oct 11, 2022 · 8 comments

Comments

@patgmiller
Copy link
Contributor

    Hi @maximbaz I've addressed all but `#1` for which I need clarification before proceeding.

Hey guys, once again - super cool job, thanks for all the work here, and I am sorry I am giving this PR less attention than it definitely deserves!

I just have some additional generic feedback from a user perspective - we can always choose to exclude this from the scope of this PR if need be +1 Sorry in advance if you already discussed those and I missed the thread...

  1. First of all, it's a lot more polished right now, feels.... complete, and behaves in exactly the way I expect it to - really cool feeling! rocket

Thanks!

  1. On the "Add credentials" page, it would be nice to remember the last selected values of password Symbols and Length - it feels intuitive that if I changed length to 20, unticked the Symbols checkbox and left the popup, next time I use "Add credentials" it would default to 20 and no symbols.

Are you saying this should persist between open / close of the browserpass popup, or just for the current open session? I imagine it will be requested at some point to have persistent defaults which can be set.

  1. Also on the "Add credentials" page: when I click on the "filename", the dropdown appears with folders - I feel like they should all have a trailing / symbol, to indicate that this is indeed a folder - when we insert the value, we do insert the trailing slash after all.

Done

  1. The above dropdown can only be completed with Enter, I actually first tried to click on an entry with a mouse, and that did nothing, so I thought the dropdown is not implemented fully yet, until I decided to try Enter... Would it be difficult to also support click to select?

Was there originally, it's fixed now.

  1. On the "Edit credentials", I somehow expect that the buttons "Save" and "Delete" should be swapped, can't really explain it, just a feeling that the "Good" action must be on the right side...? sweat_smile (also on "Add credentials").

Updated

Originally posted by @patgmiller in #290 (comment)

@patgmiller
Copy link
Contributor Author

We essentially want to the user to either remember the last used credential options (Symbols and Length) when Adding a new credential, or provide defaults which are configurable to the users desired values.

@maximbaz Please correct/clarify anything in the description which doesn't meet with your original request. Thanks

@maximbaz
Copy link
Member

Exactly right 👍 If I leave the "Add new credentials" page for example with length set to 40 (doesn't matter if at that time credentials were actually added or not), next time I open "Add new credentials", I expect to see the setup exactly as I left it last time (i.e. with length defaulted to 40 now).

@patgmiller
Copy link
Contributor Author

Exactly right 👍 If I leave the "Add new credentials" page for example with length set to 40 (doesn't matter if at that time credentials were actually added or not), next time I open "Add new credentials", I expect to see the setup exactly as I left it last time (i.e. with length defaulted to 40 now).

Do you mean even if you don't actually add any credentials? What if the user cancels and doesn't add new credentials? I was thinking only when they actually add new credentials, or to set the default length in the browser pass extension options page.

@maximbaz
Copy link
Member

Yeah let's not over complicate things 😊

I propose we don't add "length" and "symbols" to browserpass options, and just have one place where user can change generated password options instead of two (directly on the "add" screen).

And yes, if I changed password length and for one or another reason left the screen without adding password, personally I would be annoyed if my changes were not saved and I'd have to do it again!

@patgmiller
Copy link
Contributor Author

Yeah let's not over complicate things blush

I propose we don't add "length" and "symbols" to browserpass options, and just have one place where user can change generated password options instead of two (directly on the "add" screen).

And yes, if I changed password length and for one or another reason left the screen without adding password, personally I would be annoyed if my changes were not saved and I'd have to do it again!

okay, I will look at it and let you know if I have questions.

@patgmiller
Copy link
Contributor Author

@maximbaz it looks like i can safely include the length and symbols with each of the individual store settings in the local storage under the current stores key. The host app golang should ignore any json details not defined in the structs and it will avoid any merging. OR would you rather have one single key value pair for last saved length=saved_value and symbols=boolean? Basically

Option 1: Single last saved values (length, symbols) to be used for default on all new secrets. Global, not specific to any store.
Option 2: Each password-store gets it's own combination of length, symbols).

I can see value for both. Some users may want separate default values for each. I think the single global option would be less potentially confusing, but the downside is you have to change it every time if another store need a diff default. Not sure how often that will be for some, might not be an issue at all?

I think I am in favor of Option 2 because IMO I might not want my personal store to be as long as my work related one. What are your thoughts on it?

@maximbaz
Copy link
Member

I think the use-case you provided is a valid one, so I agree on option 2 👍

Idea, curious to hear your opinions, guys: remembering the last choice for length and symbols on the "add" screen is an easy productivity win if you haven't stated your desirable password length or symbols elsewhere... While I still think we shouldn't add these to the options page, what about respecting $PASSWORD_STORE_GENERATED_LENGTH and $PASSWORD_STORE_CHARACTER_SET, and maybe also having these configs in .browserpass.json file? If none of those exist, we could go with "Option 2" as per above, however if either of those is defined, then we shouldn't remember the last values for length and symbols on the "add" screen, and instead use .browserpass.json (probably should take priority, as it's per-store) or $PASSWORD_STORE_* every time 🤔 Can't decide if it's a good or a bad idea... 🤔 You guys @patgmiller and @erayd definitely have a way more complex password store structures than I do, I'm curious to hear what behavior you'd find more intuitive and useful, what would feel pleasant out of the box, without configuring Browserpass too much?

@patgmiller
Copy link
Contributor Author

I think I will almost always be in favor of supporting pass functionality, so I'm in favor of @maximbaz suggestions. However, I don't know if the existence of the env vars or browserpass json config should stop us from saving last added length / symbols. Granted that's just a nuance of how order of precedence is handled, and will mostly be determined by how it is currently handled. I haven't looked closely at this yet.

patgmiller added a commit to patgmiller/browserpass-extension that referenced this issue Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants