-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: do not hide secret value input field #3233
Conversation
You can access the deployment of this PR at https://renku-ci-ui-3233.dev.renku.ch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this, and it works as advertised, but I have a comment.
This change causes the secrets to interact with my password manager, and I'm not sure that is what we want. Before, the field was treated as just a regular text field, and therefore ignored by the password manager, and I personally prefer that UX. I'm assuming that was the motivation for the initial implementation.
In that case we would simply have a text area field and the value would be visible (i.e. we remove the clumsy blur filter). That is how some apps handle secrets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good and works as expected. 👌
Tearing down the temporary RenkuLab deplyoment for this PR. |
Fixes #3226.
/deploy #notest