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

Add f:secretTextarea UI analogous to f:password #3967

Merged
merged 1 commit into from
Apr 6, 2019

Conversation

jvz
Copy link
Member

@jvz jvz commented Apr 4, 2019

Just as f:password is provided for secrets instead of f:textinput,
this f:secretTextarea is provided for secrets instead of f:textarea
where said secrets are multiline in nature. While this component does
not provide any way to view an existing secret, it provides UIs to
add and replace secrets.

Code ported from standalone library: https://github.com/jenkinsci/lib-multiline-secrets-ui

Screenshots:
Screen Shot 2019-04-04 at 11 23 31 AM
Screen Shot 2019-04-04 at 11 23 45 AM
Screen Shot 2019-04-04 at 11 24 35 AM
Screen Shot 2019-04-04 at 11 24 58 AM
Screen Shot 2019-04-04 at 11 25 09 AM

See JENKINS-TODO.

Proposed changelog entries

  • Added Jelly UI component f:secretTextarea to /lib/form for adding and replacing multi-line secrets analogous to passwords for single-line.

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@reviewbybees @jeffret-b @daniel-beck @batmat @Wadeck @thoulen

Just as f:password is provided for secrets instead of f:textinput,
this f:secretTextarea is provided for secrets instead of f:textarea
where said secrets are multiline in nature. While this component does
not provide any way to view an existing secret, it provides UIs to
add and replace secrets.

Signed-off-by: Matt Sicker <boards@gmail.com>
@MRamonLeon
Copy link
Contributor

Good to add to the changelog entry the name of the new tag, easy to find when searching in the changelog

@jeffret-b
Copy link
Contributor

The code is good. I'm just not sure it's a good idea to duplicate it into both a library and core.

Copy link
Contributor

@Wadeck Wadeck left a comment

Choose a reason for hiding this comment

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

🐝

seems to be a perfect PR with exactly 500 lines added ;)

replaceUpdateButton();
removeSecretLegendLabel();
// fix UI bug when DOM changes
Event.fire(window, 'jenkins:bottom-sticker-adjust');
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Member

@batmat batmat left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I guess it'd be good to also add an example usage in https://github.com/jenkinsci/ui-samples-plugin?

@batmat batmat added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Apr 5, 2019
@batmat
Copy link
Member

batmat commented Apr 5, 2019

This is ready for merge. Given the small scope of this, fully new and unused yet I suppose, this is not very risky. Let's give it slightly more time, and merge it later today or tomorrow, so it goes in the next weekly.

Please anyone object before we proceed, if there's any issue with this.

Thanks everyone.

@daniel-beck
Copy link
Member

fully new and unused yet

Introduces new API to be supported forever though 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants