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(ui): Hide secret values by default #1947

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Conversation

mnonnenmacher
Copy link
Contributor

Add a new PasswordInput component that hides the value by default. The input contains a button to toggle between showing and hiding the input.

Use this component to hide the values of secrets by default.

Fixes #1940.

Add a new `PasswordInput` component that hides the value by default. The
input contains a button to toggle between showing and hiding the input.

Use this component to hide the values of secrets by default.

Fixes #1940.

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>
@Etsija
Copy link
Contributor

Etsija commented Feb 5, 2025

LGTM. If you want to get rid of some, if not all, React errors regarding uncontrolled->controlled state in the secrets forms, please see my answer in #1946.

That could also be done in a separate follow-up PR, as I believe there are many forms for which we haven't yet set defaults, and they will give more errors to dev console. Let me know what you think - we can approve this PR as-is, then fix most (if not all) React uncontrolled->controlled errors in a follow-up PR. Or, you can add fixes to secrets-specific forms in this PR, maybe in a fix commit. Then the commit could serve as an example of setting up the defaults elsewhere in the code.

@mnonnenmacher
Copy link
Contributor Author

LGTM. If you want to get rid of some, if not all, React errors regarding uncontrolled->controlled state in the secrets forms, please see my answer in #1946.

That could also be done in a separate follow-up PR, as I believe there are many forms for which we haven't yet set defaults, and they will give more errors to dev console. Let me know what you think - we can approve this PR as-is, then fix most (if not all) React uncontrolled->controlled errors in a follow-up PR. Or, you can add fixes to secrets-specific forms in this PR, maybe in a fix commit. Then the commit could serve as an example of setting up the defaults elsewhere in the code.

Thanks for doing the investigation. I found the error because I also tried a different implementation to hide secret values and I believe it did not work as expected because of this error. However, as the current implementation works without the fix I'd rather leave it as is and address the issue separately for all forms.

@mnonnenmacher mnonnenmacher added this pull request to the merge queue Feb 5, 2025
Merged via the queue into main with commit 6fdb6e0 Feb 5, 2025
27 checks passed
@mnonnenmacher mnonnenmacher deleted the password-input branch February 5, 2025 14:46
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.

Hide secret values by default
2 participants