638 Feature Request: password generator with options #664

Christian-Oleson · 2022-09-19T06:43:25Z

Pull request type

Feature Request

Please check the type of change your PR introduces:

Bugfix
Feature
UI change (please include screenshot!)
Code style update (formatting, renaming)
Refactoring (no functional changes, no api changes)
Build related changes
Documentation content changes
Internationalization and localization
Other (please describe):

What is the current behavior?

N/a

Issue Number: 638

What is the new behavior?

A new password generator feature has been made. You can select

Password Length
To include lowercase characters
To include uppercase characters
To include special characters
How many passwords to generate

Other information

My first PR to this repo
I am looking for initial feedback but may want to add Spanish and German translations to start, though I am unsure if you need all translations to be added up front.

Quality check

Before creating this PR, have you:

Followed the code style guideline as described in CONTRIBUTING.md
Verified that the change work in Release build configuration
Checked all unit tests pass

The actual options don't quite work yet.

Why is this menu still wrong...

638 - Another...

veler · 2022-09-19T21:08:21Z

Hello,
Thank you for this PR :D It looks great overall.

To give you some initial feedback and answer your questions:

No need to translate the tool yet. We use Crowdin (https://crowdin.com/project/devtoys), which will automatically pull the added strings from en-us in the main branch. That means we should first approve and merge this PR before doing the translation.
Since this is a new tool, we'd need to create a new icon for it. If you're willing to do it, you can try to follow the few instructions I left here: https://github.com/veler/DevToys/blob/main/assets/font/README.md
Initially, these instructions were for me, as I'm not a pro a making icons and fonts. If you don't feel like doing the icon, let me know and I will happily do it.
An idea of icon: how about something with *** or

This icon comes from FluentSystemIcons-Regular (glyph F59E) which we could copy and/or edit since it's under MIT license.

src/dev/DevToys.Startup/Package.appxmanifest

src/dev/impl/DevToys/DevToys.csproj

src/dev/impl/DevToys/Helpers/CryptoRandomGenerator.cs

src/dev/impl/DevToys/Strings/en-US/PasswordGenerator.resw

...impl/DevToys/ViewModels/Tools/Generators/PasswordGenerator/PasswordGeneratorToolViewModel.cs

veler · 2022-09-20T04:38:23Z

...impl/DevToys/ViewModels/Tools/Generators/PasswordGenerator/PasswordGeneratorToolViewModel.cs

+        /// <summary>
+        /// Whether the password should include numbers.
+        /// </summary>
+        private static readonly SettingDefinition<bool> Numbers


Is it really used?

I think I originally meant to add the numbers, but then realize the feature request didn't have anything about whether or not to include an option of include/exclude numbers. Would we like to do so? Or should we omit them from the options?

So, currently we have the possibility to generate password without:

lowercase letters

uppercase letters

symbols
If we also remove numbers... well there's not much to generate then 😅

But I do see the value for generating a password without numbers too.
In term of UX, I'm quite undecided at the moment, but in short, assuming we have 4 options

lowercase letters

uppercase letters

symbols

numbers
we should enforce at least 1 option, or display a message asking the user to enable at least 1 option in order to generate a password.

These options could be displayed as:

Toggle check box (like today)

List with multiple selection

List of check box

else? (I can't think of anything else)

What do you think?

It would be very easy to display a message in the output text if no options are selected. The other options would require more work, but also would be acceptable.

Could we give a try at the displaying a message?
Perhaps you can use an InfoBar like here: https://github.com/veler/DevToys/blob/2e99e04d6f363a5bddf044afc5545266be43246c/src/dev/impl/DevToys/Views/Tools/Text/RegEx/RegExToolPage.xaml#L118-L123 But display a Warning or Information instead of Error?

Another option I see (but it might be more complex too) is that when it remains only 1 option checked, we disable the checked option (the option will be checked but user won't be able to uncheck it) until another option is checked.

I'll take a look at this. It just may be awhile since I've gotten a bit busy.

...impl/DevToys/ViewModels/Tools/Generators/PasswordGenerator/PasswordGeneratorToolViewModel.cs

veler · 2022-09-20T04:40:54Z

src/dev/impl/DevToys/Views/Tools/Generators/PasswordGenerator/PasswordGeneratorPage.xaml

+                        <ex:IsCompactOverlayModeTrigger TargetElement="{x:Bind}" />
+                    </VisualState.StateTriggers>
+                    <VisualState.Setters>
+                        <Setter Target="LengthSetting.Visibility" Value="Collapsed" />


This is great, but it doesn't hide the Configuration text.
I'd suggest to give a name to the StackPanel on line 42 and collapse the whole StackPanel on line 21 (and remove lines 22 to 24).

I believe I made the modification desired here.

src/dev/shared/SharedAssemblyInfo.cs

638 - PR Concerns

Christian-Oleson · 2022-09-20T15:20:16Z

Hello, Thank you for this PR :D It looks great overall.

To give you some initial feedback and answer your questions:

No need to translate the tool yet. We use Crowdin (https://crowdin.com/project/devtoys), which will automatically pull the added strings from en-us in the main branch. That means we should first approve and merge this PR before doing the translation.

Since this is a new tool, we'd need to create a new icon for it. If you're willing to do it, you can try to follow the few instructions I left here: https://github.com/veler/DevToys/blob/main/assets/font/README.md
Initially, these instructions were for me, as I'm not a pro a making icons and fonts. If you don't feel like doing the icon, let me know and I will happily do it.
An idea of icon: how about something with *** or

This icon comes from FluentSystemIcons-Regular (glyph F59E) which we could copy and/or edit since it's under MIT license.

I'll have to see if I can find another computer. FontForge is not liked by my machine it seems. I'll work to get it on another and see if I can follow the setup to generate a

Christian-Oleson · 2022-10-09T17:50:39Z

FYI, I'm still working this, just went out on parental leave last week, so it will take a bit longer than expected.

veler · 2022-10-09T18:48:03Z

FYI, I'm still working this, just went out on parental leave last week, so it will take a bit longer than expected.

@Christian-Oleson no worries :) I can address the remaining feedback myself if you'd like too.
Also, congratulations for the newborn! 💯

nj · 2022-10-31T09:04:58Z

Might be good to extend further later on, perhaps with inspiration by https://randomkeygen.com/ (on GitHub at https://github.com/circlecell/randomkeygen.com, MIT licensed).

Christian-Oleson · 2022-11-05T14:39:07Z

FYI, I'm still working this, just went out on parental leave last week, so it will take a bit longer than expected.

@Christian-Oleson no worries :) I can address the remaining feedback myself if you'd like too.

Also, congratulations for the newborn! 💯

Thank you! It may be best to have you finish. I'm still out and a bit busy, so it may be best to get this released instead of having folks wait on me eventually completing this on my end.

veler · 2023-07-20T15:28:34Z

Closing this PR as the tool got completed into #873 . Thank you @Christian-Oleson and @micahmo for this contribution! :)

Christian-Oleson added 4 commits September 18, 2022 23:21

Merge remote-tracking branch 'upstream/main' into main

fd84ee4

638 - Add a Password Generator

7657b76

The actual options don't quite work yet.

638 - Minor fix

a278ebc

Why is this menu still wrong...

638 - Ready for review

1b35ce3

Christian-Oleson mentioned this pull request Sep 19, 2022

Feature Request: password generator with options #638

Closed

638 - Forgot this?

a4d1406

638 - Another...

veler linked an issue Sep 19, 2022 that may be closed by this pull request

Feature Request: password generator with options #638

Closed

veler reviewed Sep 20, 2022

View reviewed changes

Christian-Oleson added 4 commits September 20, 2022 09:32

Merge remote-tracking branch 'upstream/main' into main

2b19417

Merge branch 'main' into 638-feature-request-password-generator-w

ff39c3e

638 - PR Concerns

7bb8fbb

638 - PR Conccerns

4dfc28a

638 - PR Concerns

Christian-Oleson added 3 commits September 27, 2022 10:41

638 - PR Concerns

39de033

Merge remote-tracking branch 'upstream/main' into main

3f30ac5

Merge branch 'main' into 638-feature-request-password-generator-w

b85bf8f

Christian-Oleson added 2 commits October 11, 2022 22:39

Merge remote-tracking branch 'upstream/main' into main

2f70a9c

Merge branch 'main' into 638-feature-request-password-generator-w

20fb5ca

Fixed merge conflict

1c3aa38

micahmo mentioned this pull request Jul 5, 2023

Part 2: 638 Feature Request: password generator with options #873

Merged

18 tasks

veler closed this Jul 20, 2023

Christian-Oleson deleted the 638-feature-request-password-generator-w branch July 20, 2023 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

638 Feature Request: password generator with options #664

638 Feature Request: password generator with options #664

Christian-Oleson commented Sep 19, 2022 •

edited

Loading

veler commented Sep 19, 2022

veler Sep 20, 2022

Christian-Oleson Sep 20, 2022

veler Sep 20, 2022

Christian-Oleson Sep 20, 2022

veler Sep 20, 2022

Christian-Oleson Sep 27, 2022

veler Sep 20, 2022

Christian-Oleson Sep 25, 2022

Christian-Oleson commented Sep 20, 2022

Christian-Oleson commented Oct 9, 2022

veler commented Oct 9, 2022

nj commented Oct 31, 2022

Christian-Oleson commented Nov 5, 2022

veler commented Jul 20, 2023

638 Feature Request: password generator with options #664

638 Feature Request: password generator with options #664

Conversation

Christian-Oleson commented Sep 19, 2022 • edited Loading

Pull request type

What is the current behavior?

What is the new behavior?

Other information

Quality check

veler commented Sep 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Christian-Oleson commented Sep 20, 2022

Christian-Oleson commented Oct 9, 2022

veler commented Oct 9, 2022

nj commented Oct 31, 2022

Christian-Oleson commented Nov 5, 2022

veler commented Jul 20, 2023

Christian-Oleson commented Sep 19, 2022 •

edited

Loading