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

feat: disable autocomplete on other form fields #2595

Merged
merged 2 commits into from
Jul 28, 2023
Merged

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Jul 27, 2023

Describe the changes you have made in this PR

don't allow password manager to fill password field other then fields on unlock screen add types to text fields

Link this PR to an issue [optional]

Fixes #2489 new pr for #2497

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

How has this been tested?

manually checking auto complete on firefox

Checklist

  • Self-review of changed code
  • Manual testing
  • Added automated tests where applicable
  • Update Docs & Guides
  • For UI-related changes
  • Darkmode
  • Responsive layout

don't allow password manager to fill password field other then fields on unlock screen
add types to text fields
signed-off-by: pavan joshi <pavanj914@gmail.com>
@@ -126,6 +126,7 @@ function Send() {
<div className="pt-4">
<TextField
id="invoice"
type="text"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add the type everywhere? It's "text" by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have type added as text at few of the text field and few of them doesn't contain them. so shall we remove type field at all the fields containing type="text"

it's better to be consistent!

Copy link
Contributor Author

@pavanjoshi914 pavanjoshi914 Jul 27, 2023

Choose a reason for hiding this comment

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

having type explicitly in the code would be better and readable isn't it?

@reneaaron reneaaron changed the title feat: disable autocomplet on other form fields feat: disable autocomplete on other form fields Jul 27, 2023
@@ -497,6 +498,7 @@ function LNURLPay() {
<div className="mt-4">
<TextField
id="name"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think type text is default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have type="text" at few of the text field and few of them doesn't contain them. it's better to be consistent!

shall we remove it from everywhere or add it everywhere? isn't this better and readable to have type explicitly defined

@@ -175,6 +176,7 @@ export default function ConnectLnd() {
<div>
<TextField
id="macaroon"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make this also a password field

@@ -133,6 +133,7 @@ export default function ConnectMyNode() {
<div className="mt-6">
<TextField
id="lndconnect"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make this also a password field

@@ -164,6 +165,7 @@ export default function ConnectRaspiBlitz() {
<div>
<TextField
id="macaroon"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make this also a password field

@@ -136,6 +136,7 @@ export default function ConnectStart9() {
<div className="mt-6">
<TextField
id="lndconnect"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make this also a password field.

lndconnect holds secrets and thus we can handle it as a password field I guess.

@@ -133,6 +133,7 @@ export default function ConnectUmbrel() {
<div className="mt-6">
<TextField
id="lndconnect"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make this also a password field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG] Unlock password gets used as nostr private key
3 participants