[JENKINS-69787] Create new dismiss component #7265

CrystallDEV · 2022-10-15T12:48:38Z

…in messages

See JENKINS-69787.

Testing done

Manually tested with local Jenkins instance

- Executed all existing tests

Proposed changelog entries

Added new dismiss (jelly) component which uses the (new) introduced jenkins-button css class and use it in several admin notifications

Proposed upgrade guidelines

N/A

Submitter checklist

The Jira issue, if it exists, is well-described.
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
- Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
[] There is automated testing or an explanation as to why this change has no tests.
[] New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
For dependency updates, there are links to external changelogs and, if possible, full differentials.
For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

N/A

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Proper changelog labels are set so that the changelog can be generated automatically.
If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

…in messages

NotMyFault

Hey,

thanks for the PR!

If an interface already has a button, they are not aligned:

core/src/main/resources/lib/form/dismiss.jelly

timja · 2022-10-19T18:36:13Z

I was expecting something more like this component that you can use to remove a step / repeatable section,

e.g. from shell step:

daniel-beck · 2022-10-19T20:45:08Z

core/src/main/resources/lib/form/dismiss.jelly

+      When you have more than one dismiss button on the form, this can be used to determine
+      which button is pressed, as the server will get a parameter by this name.
+    </s:attribute>
+    <s:attribute name="tooltip" use="required">


Needs to specify the expectation around HTML content.

CrystallDEV · 2022-10-20T09:19:33Z

Thanks everyone for the reviews. Ill fix the leftovers within the next week when I hopefully have time

github-actions · 2022-11-14T16:06:05Z

Please take a moment and address the merge conflicts of your pull request. Thanks!

daniel-beck

Noting the requirement of #7265 (review)

NotMyFault · 2023-01-13T17:55:18Z

Hey @CrystallDEV,

would you mind addressing the outlined feedback above, please?

Thanks in advance!

CrystallDEV · 2023-02-24T14:43:10Z

@NotMyFault @daniel-beck @uhafner I finally managed to find time and motivation to fix the PR. Looks good on my machine 😄

uhafner

Thanks for addressing most of the feedback! Can you please also update the screenshots in the PR description?

uhafner · 2023-02-27T08:39:19Z

core/src/main/resources/lib/form/dismiss-button.jelly

+      which button is pressed, as the server will get a parameter by this name.
+    </s:attribute>
+    <s:attribute name="value">
+      The tooltip text of the dismiss button.


Make clear that HTML is not allowed in tooltip.

uhafner · 2023-02-27T08:42:01Z

core/src/main/resources/lib/form/dismiss-button.jelly

+  </s:documentation>
+  <button
+    name="${attrs.name ?: 'Dismiss'}"
+    tooltip="${attrs.value ?: '%Dismiss'}"


Wouldn't it make more sense to have no tooltip if not given? It would not help if it is the same as the name. (If not you should at least make it consistent to the name). Is the % a typo?

% is for internationalization

Then it should be in name as well?

name is used as part of form submission so definitely should not change in different locales. tooltip is what's displayed to users (although as you said should probably be omitted if it's the same as attrs.value as it provides no value)

So anything I should change?

Probably should remove the tooltip I think. Or make it customise-able without also changing value.

e.g.
tooltip="${attrs.tooltip}"

I think would work

CrystallDEV · 2023-02-27T14:00:59Z

Updated Screenshot

github-actions · 2023-03-20T09:47:58Z

Please take a moment and address the merge conflicts of your pull request. Thanks!

JENKINS-69787: Create new dismiss component and use it in several adm…

174ac49

…in messages

NotMyFault reviewed Oct 15, 2022

View reviewed changes

NotMyFault added web-ui The PR includes WebUI changes which may need special expertise rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted needs-security-review Awaiting review by a security team member labels Oct 15, 2022

NotMyFault requested review from a team October 15, 2022 16:26

NotMyFault added the hacktoberfest-accepted label Oct 15, 2022

NotMyFault changed the title ~~JENKINS-69787: Create new dismiss component and use it in several adm…~~ [JENKINS-6978] Create new dismiss component Oct 15, 2022

uhafner suggested changes Oct 15, 2022

View reviewed changes

jenkinsci deleted a comment from uhafner Oct 15, 2022

daniel-beck reviewed Oct 19, 2022

View reviewed changes

daniel-beck changed the title ~~[JENKINS-6978] Create new dismiss component~~ [JENKINS-69787] Create new dismiss component Oct 20, 2022

github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Nov 14, 2022

daniel-beck requested changes Nov 18, 2022

View reviewed changes

daniel-beck removed the needs-security-review Awaiting review by a security team member label Nov 18, 2022

github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Jan 3, 2023

NotMyFault added the needs-comments-resolved Comments in the PR need to be addressed before it can be considered for merging label Jan 13, 2023

CrystallDEV added 2 commits February 24, 2023 14:44

Merge branch 'jenkinsci:master' into fix/JENKINS-69787

f7ffc7b

JENKINS-69787: Apply requested PR review changes

9b2e840

CrystallDEV requested review from daniel-beck and uhafner and removed request for daniel-beck and uhafner February 24, 2023 14:36

CrystallDEV requested a review from NotMyFault February 24, 2023 14:37

uhafner suggested changes Feb 27, 2023

View reviewed changes

github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Mar 20, 2023

basil removed the hacktoberfest-accepted label Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-69787] Create new dismiss component #7265

[JENKINS-69787] Create new dismiss component #7265

CrystallDEV commented Oct 15, 2022 •

edited

Loading

NotMyFault left a comment

timja commented Oct 19, 2022

daniel-beck Oct 19, 2022

CrystallDEV commented Oct 20, 2022

github-actions bot commented Nov 14, 2022

daniel-beck left a comment

NotMyFault commented Jan 13, 2023

CrystallDEV commented Feb 24, 2023

uhafner left a comment

uhafner Feb 27, 2023

uhafner Feb 27, 2023

timja Feb 27, 2023 •

edited

Loading

uhafner Feb 27, 2023

timja Feb 27, 2023 •

edited

Loading

CrystallDEV Feb 27, 2023

timja Feb 27, 2023

CrystallDEV commented Feb 27, 2023

github-actions bot commented Mar 20, 2023

[JENKINS-69787] Create new dismiss component #7265

Are you sure you want to change the base?

[JENKINS-69787] Create new dismiss component #7265

Conversation

CrystallDEV commented Oct 15, 2022 • edited Loading

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

NotMyFault left a comment

Choose a reason for hiding this comment

timja commented Oct 19, 2022

daniel-beck Oct 19, 2022

Choose a reason for hiding this comment

CrystallDEV commented Oct 20, 2022

github-actions bot commented Nov 14, 2022

daniel-beck left a comment

Choose a reason for hiding this comment

NotMyFault commented Jan 13, 2023

CrystallDEV commented Feb 24, 2023

uhafner left a comment

Choose a reason for hiding this comment

uhafner Feb 27, 2023

Choose a reason for hiding this comment

uhafner Feb 27, 2023

Choose a reason for hiding this comment

timja Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

uhafner Feb 27, 2023

Choose a reason for hiding this comment

timja Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

CrystallDEV Feb 27, 2023

Choose a reason for hiding this comment

timja Feb 27, 2023

Choose a reason for hiding this comment

CrystallDEV commented Feb 27, 2023

github-actions bot commented Mar 20, 2023

CrystallDEV commented Oct 15, 2022 •

edited

Loading

timja Feb 27, 2023 •

edited

Loading

timja Feb 27, 2023 •

edited

Loading