-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
move the header parts in a taglib #9223
Conversation
the taglib allows plugins to make use of the searchbox and the login stuff while easily being able to modify all the other aspects of the header.
<h:logo/> | ||
<h:searchbox/> | ||
<h:login/> |
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 is now public API. Are we confident a redesign doesn't just randomly change this anytime soon?
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.
Any substantial change to the header would require a change of the header version in the PartialHeader so that plugins need to react. I might need to stick to the PartialHeader in the PR of my plugin (which is so far the only implementation of a header) https://github.com/jenkinsci/customizable-header-plugin/blob/295683015391e800be41c568b42ac80f6fd7c61b/src/main/java/io/jenkins/plugins/customizable_header/headers/LogoHeader.java#L9
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.
Makes sense, downstream changes look good
/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! |
the taglib allows plugins to make use of the searchbox and the login stuff while easily being able to modify all the other aspects of the header.
Testing done
Manually verified that the header is not modified. Also in the downstream PR using the taglib works without problems.
see jenkinsci/customizable-header-plugin#101
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Before the changes are marked as
ready-for-merge
:Maintainer checklist