[JENKINS-75683] re-introduce actions for Jenkins with custom action.jelly#10678
Merged
MarkEWaite merged 6 commits intojenkinsci:masterfrom Jun 24, 2025
Merged
[JENKINS-75683] re-introduce actions for Jenkins with custom action.jelly#10678MarkEWaite merged 6 commits intojenkinsci:masterfrom
MarkEWaite merged 6 commits intojenkinsci:masterfrom
Conversation
jtnord
commented
May 27, 2025
Contributor
|
FYI PR To fix the styling jtnord#2 |
14 tasks
14 tasks
Member
Author
|
Seems to be an issue with the specific action that is trying to infer things from the current URL, and making assumptions that no longer hold true. |
This PR distinguishes between primary and secondary Actions for the header. Primary actions are always shown, and secondary actions are by default in the Hamburger menu. When the current page is the page for the secondary action (or a parent thereof) the action is put in the main header action list and not in the hamburger menu.
599134e to
3bc9a6d
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jtnord
added a commit
to jtnord/purge-build-queue-plugin
that referenced
this pull request
Jun 19, 2025
The code was incorrectly checking the permission ${it.ADMINISTER}
however it when vewing the root of Jenkins (and other views) is an
instance of AllView. As such the permission check failed silently and
was then not shown.
jenkinsci/jenkins#10678 sets ${it} to Jenkins
now, and then this causes the incorrect permission check to blow up as
it is a single permission that is set for permissions (which expects an
array).
This fixes the issue and actually removes the permission check from the
task, as the permission is already checked in the layout.
We now take advantage of the Manage permission also so that users
without Jenkins.ADMINISTER but with JENKINS.MANAGE can clear the build
queue.
This was referenced Jun 19, 2025
Merged
NotMyFault
pushed a commit
to jenkinsci/purge-build-queue-plugin
that referenced
this pull request
Jun 19, 2025
* bump to recent parent pom
* Compatability with Future Jenkins version
The code was incorrectly checking the permission ${it.ADMINISTER}
however it when vewing the root of Jenkins (and other views) is an
instance of AllView. As such the permission check failed silently and
was then not shown.
jenkinsci/jenkins#10678 sets ${it} to Jenkins
now, and then this causes the incorrect permission check to blow up as
it is a single permission that is set for permissions (which expects an
array).
This fixes the issue and actually removes the permission check from the
task, as the permission is already checked in the layout.
We now take advantage of the Manage permission also so that users
without Jenkins.ADMINISTER but with JENKINS.MANAGE can clear the build
queue.
…elly actions without a URL but a custom action.jelly where not shown correctly in the header. This re-introduces them and allows custom rendering of actions again (by using an action.jelly). adjust the task taglib so that it renders in the style of the header when in the header. Nested tasks work, but when expanded the original looses its tooltip. move subtasks to the dropdown use tasks class to fix layout fix scss formatting
jtnord
added a commit
to jtnord/acceptance-test-harness
that referenced
this pull request
Jun 19, 2025
Closed
6 tasks
MarkEWaite
approved these changes
Jun 23, 2025
scherler
approved these changes
Jun 23, 2025
MarkEWaite
approved these changes
Jun 23, 2025
Contributor
MarkEWaite
left a comment
There was a problem hiding this comment.
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.
/label ready-for-merge
Contributor
|
Please take a moment and address the merge conflicts of your pull request. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JENKINS-75683
Actions without a URL but a custom
action.jellywere not shown correctly in the header.NB: this is built on top of #10729 adn the changes for this PR are all contained in b091ea21533865f26ee747bec2930bb1f730465d
This re-introduces them and allows custom rendering of actions again (by using an action.jelly).
Actions that require a confirmation (and post) work.
Things still to do,
* task taglib needs to be adjusted to style correctly when tasks are in the header~ ** nested tasks need to be handled appropriately (they are included only in sub pages, but see previous bullet point)~
* remove the url hack.* Fix the nested tasks so that the parent still contains the correct tooltip.* use the dropdown for nested elements..?* the task styling in the dropdown is oh so wrong...* check the rendering when inside the "overflow" icon dropdown.* run ATHScreen Shots
Note
the after screenshots include the hamburger introduced in https://github.com/jenkinsci/jenkins/pull/10729which is a requirement for this change.
Before
with overflow menu:


without overflow menu:
issues:
action.jellythat would hide the actions (but they show up).action.jellythat includes sub tasks that should be shown.After
no current action:

no current action expanded hamburger:

current action with subtasks:

current action with hamburger: (action removed from hamburger)

Support for tasks that require confirmation:

The above demonstrate that an
Actionwith anaction.jellyare now handled correctly.action.jellysays they should be regardless of the lack of an icon from the java side.action.jellyno longer showSee JENKINS-75683.
Testing done
Proposed changelog entries
RootActions that used custom rendering (action.jelly) are once again visible in the header and behave appropriately.RootActions that use jelly to show/hide are correctly hidden in the headerRootActionswith sub tasks now show the subtasks as a dropdown when in the correct contextProposed 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
@mention
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).