-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Add badges to build page #10651
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
base: master
Are you sure you want to change the base?
Add badges to build page #10651
Conversation
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.
This looks weird with the usual (everywhere else except badge
and groovy-buildstep
) badges that are just an icon.
Plus the tooltip is wrong for this one in core, the sidepanel includes the cause:
(forgot to set it
or similar?)
IMO: If an action should appear on the project page, there should be such an action. Reusing badges just for convenience in badge
plugin seems too focused on the existing implementation.
Can't really argue with that. Maybe adding some sort of container would help to make the icons not look so out of place.
The idea is to mirror the information from the build history here, hence a new action would defeat that purpose. It's less about convenience but consistency. Or is there any reason to not have that information here? |
Fixed in 73d4080 |
I disagree with the idea, that's the point of my feedback. If the information should appear on this page as well, then it should be such an action added to the build. For example https://plugins.jenkins.io/buildtriggerbadge/ only makes sense in the widget, because causes are already shown on the main build page. Badge plugin could do this, and it's not clear to me why it's not being changed to do this. What do we gain from this change to core behavior?
These look a lot nicer. To the right of the build number/name might also work. |
Understood. I also kind of disagree with your feedback, but you do raise a fair point.
Like how? The only extension point I can think of here is to have a
I tried that but was not really happy with the result, so I scratched it. |
Looking at the plugin, |
These are implementation choices and could easily be done differently by the plugin.
Others may see this as allowing fine-grained control of what shows where, and this change would result in a confusing API ("badges" are actually "badge + summary" and summary is just summary). |
I don't think this is the case as the layout is controlled by the enclosing container which resides in core.
Not sure if I fully agree on that. I think it's more about displaying badges not only in the build history widget but also on the build summary page. Similar to the build status, number or execution time. |
Add badge actions to the build page.
Following a discussion with @janfaracik and @timja in jenkinsci/badge-plugin#263 I propose to add badges (provided by
BadgeActions
) to the build page. In my opinion it only makes sence to have the information of the badges displayed here as well rather then having it only in the build history.Full disclaimer: I am far from being a UI/UX expert, but want to push this topic forward. So any feedback is highly welcome!
Testing done
See before and after screenshots:
Before
After
Proposed changelog entries
Proposed changelog category
/label rfe,web-ui
Proposed upgrade guidelines
N/A
Submitter checklist
@Restricted
or have@since TODO
Javadocs, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
, if applicable.eval
to ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@jenkinsci/sig-ux @jenkinsci/core-pr-reviewers
Before the changes are marked as
ready-for-merge
:Maintainer checklist
upgrade-guide-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidate
to be considered (see query).