Introduce experimental API for adding actions to experimental Run UI#11204
Conversation
3c16200 to
db697ec
Compare
| return null; | ||
| } | ||
|
|
||
| default Group getGroup() { |
There was a problem hiding this comment.
Should this be part of Action? Or into its own interface?
| return null; | ||
| } | ||
|
|
||
| default Group getGroup() { |
There was a problem hiding this comment.
Is this the best way of grouping/ordering items?
| @@ -106,102 +111,56 @@ public void generateResponse(StaplerRequest2 req, StaplerResponse2 rsp, Object o | |||
| } | |||
|
|
|||
| public ContextMenu add(String url, String text) { | |||
There was a problem hiding this comment.
This file needs validating (maybe a clean up if possible without breaking plugins...)
| import jenkins.model.TransientActionFactory; | ||
| import jenkins.model.menu.Group; | ||
| import jenkins.security.stapler.StaplerNotDispatchable; | ||
| import org.apache.commons.lang.StringUtils; |
There was a problem hiding this comment.
Usage of deprecated commons-lang2
There was a problem hiding this comment.
Its used in 2 other places in core, should be banned via e.g checkstyle if we don't want it reintroduced.
timja
left a comment
There was a problem hiding this comment.
Given the new APIs are marked as Beta lets proceed so we can evolve the API as required.
Any feedback on the API is welcomed, this has been open for comment for long enough though.
/label ready-for-merge
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.
Thanks!
|
Likely cause of #26639. |
| * | ||
| * @return the group of this item | ||
| */ | ||
| default Group getGroup() { |
There was a problem hiding this comment.
If the new API is “experimental”, should the new signatures not be marked with @Restricted(Beta.class)?
There was a problem hiding this comment.
could be, the new classes have been marked as beta.
| return semantic; | ||
| } | ||
|
|
||
| public MenuItem(String url, String icon, String displayName) { |
There was a problem hiding this comment.
Similar to #11204 (comment). FYI this broke a CloudBees CI plugin. @jenkinsci/company-cloudbees-developers (the membership is outdated BTW) ought to be checking proposed incompatible API changes against CloudBees CI as noted here.
With jenkinsci/jenkins#11204 a new method was added to Action with name `getEvent`. This clashes with the getEvent method in BadgeAction. As the getEvent in BadgeAction is not used outside of tests and there are no users of BadgeAction in other plugins it should be safe to rename the method. Also rename the setter to by in sync, which is also not used anywhere. Persistence via XStream doesn't rely on the methods but works directly with the fields.


This PR introduces an experimental API for adding actions to the experimental Run UI - actions are visible in the app bar and its overflow menu.
WIP screenshot
I've been wanting to get this done for a long (long) time now, the intention is to provide a simple to use API for developers to add simple and complex actions to our Actionable objects.
An action can be as simple as taking the user to a new page, or it can be as complex as executing arbitrary JS or displays dialogs and modals.
Testing done
Proposed changelog entries
Proposed changelog category
/label web-ui,rfe
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@jenkinsci/sig-ux @daniel-beck
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).