Skip to content
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

[JENKINS-72947] Support symbols in TopLevelItemDescriptor#getIconClassName() #9127

Merged
merged 4 commits into from
Apr 14, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Apr 4, 2024

When adding a new Item via newJob the corresponding Descriptor can now make use of symbols

As a sample I modified the freestyle project to use a new symbol (svg source: https://www.svgrepo.com/svg/151600/package)

The image shows the old page layout with manual modifying the css in the browser
image

Alternate icon from @janfaracik with new layout
image

See JENKINS-72947.

Testing done

Manual testing opening newJob in root and in a folder
Added a new test that check svg is inserted and has proper class icon-xlg
Adjusted test for old iconClassName

Proposed changelog entries

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

…sName()

When adding a new Item via `newJob` the corresponding Decriptor can now
make use of symbols
@janfaracik
Copy link
Contributor

Worked on an icon for freestyle projects a while back - posting it here as an alternative:

image

https://github.com/janfaracik/jenkins/blob/c029ea4b4cfbea107fc904a4413613886f80090f/war/src/main/resources/images/symbols/freestyle-project.svg

@mawinter69 mawinter69 marked this pull request as ready for review April 7, 2024 20:42
@timja timja added rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise labels Apr 7, 2024
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to send some PRs to other item types as well for consistency.

These are the ones I see in my test setup and would likely cover most of them:

image

@NotMyFault NotMyFault changed the title [JENKINS-72947] Support symbols in TopLevelItemDescriptor#getIconClassName() [JENKINS-72947] Support symbols in TopLevelItemDescriptor#getIconClassName() Apr 7, 2024
Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@NotMyFault
Copy link
Member

/label ready-for-merge


This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback.
Please see the merge process documentation for more information about the merge process.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants