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-73586] Fix New Item page layout if no icon is defined for an item type #9520

Merged
merged 4 commits into from
Aug 9, 2024

Conversation

scherler
Copy link
Contributor

@scherler scherler commented Aug 2, 2024

#9111

introduced a new styling however it broke the styling of the default icon generation in case there is no icon defined. as shown in 1aff74c

image

After fix
image

Testing done

Interactive checks

Proposed changelog entries

  • Fix "New Item" page layout if no icon is defined for an item (regression in 2.453).

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

Signed-off-by: Thorsten Scherler <scherler@gmail.com>
Signed-off-by: Thorsten Scherler <scherler@gmail.com>
Signed-off-by: Thorsten Scherler <scherler@gmail.com>
@scherler scherler marked this pull request as ready for review August 2, 2024 23:18
@janfaracik
Copy link
Contributor

Thanks!

Out of curiosity, is there a situation where an item type wouldn't provide an icon?

@mawinter69
Copy link
Contributor

e.g. the Coordinator plugin implements a new project type but is not providing an icon or image. So it displays wrong:
image

@janfaracik
Copy link
Contributor

e.g. the Coordinator plugin implements a new project type but is not providing an icon or image. So it displays wrong: image

Thanks!

Any thoughts on this - Depending on how many plugins there are that don't provide an icon, could we instead update the plugins to include an icon rather than maintaining this logic in core?

@scherler
Copy link
Contributor Author

scherler commented Aug 5, 2024

There are some situations in which you cannot provide an icon without a bigger rewrite, IMHO it is there, and removing such functionality is problematic since we have no way of knowing how many plugins depend on this.

@scherler scherler changed the title [new_item_page_fix] show problem [new_item_page_fix] fix layout if no icon is defined Aug 6, 2024
@basil basil added the bug For changelog: Minor bug. Will be listed after features label Aug 7, 2024
@timja
Copy link
Member

timja commented Aug 7, 2024

/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!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 7, 2024
@daniel-beck daniel-beck changed the title [new_item_page_fix] fix layout if no icon is defined Fix New Item page layout if no icon is defined for an item type Aug 7, 2024
@daniel-beck
Copy link
Member

This should have a Jira issue for backporting into LTS.

@scherler scherler changed the title Fix New Item page layout if no icon is defined for an item type [JENKINS-73586] Fix New Item page layout if no icon is defined for an item type Aug 8, 2024
@scherler
Copy link
Contributor Author

scherler commented Aug 9, 2024

@daniel-beck I created JENKINS-73586

@timja timja merged commit e763a49 into jenkinsci:master Aug 9, 2024
16 checks passed
@scherler scherler deleted the new_item_page_fix branch August 12, 2024 07:26
NotMyFault pushed a commit to NotMyFault/jenkins that referenced this pull request Aug 14, 2024
… item type (jenkinsci#9520)

* [new_item_page_fix] show problem

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

* [new_item_page_fix] fix problem

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

* [new_item_page_fix] revert freestyle hack

Signed-off-by: Thorsten Scherler <scherler@gmail.com>

---------

Signed-off-by: Thorsten Scherler <scherler@gmail.com>
(cherry picked from commit e763a49)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants