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

Change behavior of "hide password" checkbox #44

Closed
bndw opened this issue Jul 14, 2021 · 9 comments
Closed

Change behavior of "hide password" checkbox #44

bndw opened this issue Jul 14, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@bndw
Copy link
Owner

bndw commented Jul 14, 2021

Change the behavior of the Hide password checkbox as follows:

  1. Hide password checkbox hides the password textarea with display: none. This would make is immediately invisible to the user (expected) as well as the printout.
  2. Selecting "None" encryption automatically checks hide password. This will trigger (1) and hide the password textarea
  3. Changing the encryption mode from "None" to something else automatically unchecks hide password
@Mouradouchane
Copy link

Hi man , is that issue good as first one .
i think i can help if you want

@noahhefner
Copy link
Contributor

So I think there's two possible approaches here: 1) Automatically check the hide password option on selection of "None" encryption type as descried in this issue or 2) hide the password entry box altogether on selection of the "None" encryption type. Option 2) seems to be a more natural implementation IMO. Thoughts?

@bndw
Copy link
Owner Author

bndw commented Jul 15, 2021

@noahhefner You beat me to it! One caveat-- There is the use-case where you have a password but you might not want it on the printout. I think we need the hide password checkbox for that.

What if we changed the behavior as follows:

  • Hide password checkbox hides the password textarea with display: none. This would make is immediately invisible to the user (expected) as well as the printout.
  • Selecting "None" encryption automatically checks hide password
  • Changing the encryption mode from "None" to something else automatically unchecks hide password

I think that would result in the most intuitive behavior while covering all use-cases. Thoughts?

@noahhefner
Copy link
Contributor

noahhefner commented Jul 15, 2021

Yeah that sounds good to me! So to clarify, selecting "None" encryption would automatically check hide password and hide the password textarea.

Also, when selecting "None" encryption, the network.password field remains unchanged. In my testing, the password will actually change the QR code that is generated for None encryption. So assuming that we want to have no password for None encryption, we will need to clear network.password when upon selection of None encryption. Something like this:

setNetwork({ ...network, password: '' })

EDIT: This seems like an edge case so it's not imperative that this is implemented, just something I found while testing.

@bndw
Copy link
Owner Author

bndw commented Jul 15, 2021

So to clarify, selecting "None" encryption would automatically check hide password and hide the password textarea.

Correct

we will need to clear network.password when upon selection of None encryption

Nice catch, that sounds like a bug. Clearing the password as you described should resolve it. (I'm not sure if it matters to the end device, but it's something worth cleaning up either way)

EDIT: I've updated the first comment in this issue to reflect. I've also opened a separate issue for clearing the password value on "None" encryption.

@bndw bndw changed the title Automatically check "hide password" with "None" encryption Change behavior of "hide password" checkbox Jul 15, 2021
@bndw
Copy link
Owner Author

bndw commented Jul 15, 2021

@Mouradouchane If you're familiar with React and JavaScript, this could be a reasonable first issue. Alternatively, #46 would be a great first issue.

@Mouradouchane
Copy link

@bndw yeah i'am familiar with javascript , but i don't know anything about react
last things , is there any installation instruction or guideline

@bndw
Copy link
Owner Author

bndw commented Jul 15, 2021

@Mouradouchane Just added some in #47

@bndw bndw added the enhancement New feature or request label Jul 15, 2021
@Mouradouchane
Copy link

Hi @bndw , I just want to tell you, I did my best for hours, trying to contribute to your great project, but I couldn't do anything due to my lack of knowledge of "React"
I'm sorry if I'm anoying to you, good luck

bndw pushed a commit that referenced this issue Jul 16, 2021
* Changes behavior of password textarea when None encryption is selected

* prettier

* password text area hides on checking hide password tikbox

* className re-structuring, changed Hide Password
@bndw bndw closed this as completed Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants